Fix NotebookPath equality/hash contract for equal relative paths - #330
Open
Flerpharos wants to merge 1 commit into
Open
Fix NotebookPath equality/hash contract for equal relative paths#330Flerpharos wants to merge 1 commit into
Flerpharos wants to merge 1 commit into
Conversation
Two equal unanchored relative paths compared unequal while hashing equal:
`__hash__` fell back to the raw normalized state on PathError, but `__eq__`
returned False on the same PathError, so `NotebookPath("a/b") == NotebookPath("a/b")`
was False while both hashed equal -- breaking dict/set membership.
Give both a shared `_comparison_key()` (resolved path when resolvable, else
the raw `(absoluteness, segments)` state). Resolvable paths are always
absolute, so an unanchored relative path can never compare equal to an
absolute one; only genuinely equal relative paths now compare equal.
Fixes #319.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
This attempt to fix breaks the fundamental equality system where different paths that go to the same place are equal. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the HIGH audit finding #319. Two equal unanchored relative
NotebookPaths compared unequal while hashing equal:__hash__fell back to(self._absolute, tuple(self._parts))whenresolve()raisedPathError(an unanchored relative path can't resolve) — so equal relative paths hashed the same.__eq__returnedFalseon that samePathError— so they compared unequal.Result:
NotebookPath("foo/bar") == NotebookPath("foo/bar")wasFalse,b in {a}wasFalse, and equal relative paths became distinct dict/set keys.Fix
Both
__eq__and__hash__now use a shared_comparison_key()— the resolved path when resolvable, else the raw normalized(absoluteness, segments)state. Sinceresolve()always yields an absolute path, an unanchored relative path can never key-match an absolute one, so only genuinely equal relative paths compare equal. No other equality changes.Verification
New test
test_notebook_path_equal_relative_paths_are_equal_and_hashable(==, hash,in {…}, set dedup, dict lookup, plus non-equality preserved). Full suite: 431 passed, 1 skipped; ruff/pyright clean.Fixes #319.
🤖 Generated with Claude Code