fix: flag a closure that resolves but never leaves its own file (#63) - #69
Merged
Merged
Conversation
A changed file's seeds can map to real symbols whose closure still stays confined to that one file — indistinguishable today from a confident, correct NONE. select() now checks this per changed file (distinct from recall_degraded, which covers unmapped seeds) and reports it as closure_confined plus a warning/NOTE.
- closure_files() routes ids through a TEMP TABLE like impacted_closure does, instead of one bound placeholder per id — a wide fan-out could exceed SQLite's variable ceiling and raise. - Reuse the already-computed `impacted` closure when a diff seeds exactly one file, instead of re-running the same recursive traversal. - hook.py's ledger record and selectivity.py's per-commit sweep now also track closure_confined, so it isn't invisible to the project's own measurement tooling the way recall_degraded already is.
- Reword the confinement NOTE/warning: it no longer implies the whole answer is NONE when other changed files did select journeys — it's scoped to that one file's own contribution. - Verify the single-file reuse shortcut by value (file_seeds == seeds) instead of inferring it from len(seeds_by_file) == 1, so a future seed source can't silently desync the two. - Extract the TEMP TABLE load duplicated between impacted_closure and closure_files into one _load_id_temp_table helper. - Document the accepted per-file traversal cost on wide diffs instead of silently leaving it unexplained.
- hook.py's pre-push render() gave closure_confined its own uncapped line, like recall_degraded already has -- riding the MAX_WARNINGS=3 channel could push the one signal this issue exists to surface off the actual push output. - A rename that also carries edited hunks landed the new path in both ranges and whole_files; the whole-file loop was unconditionally overwriting seeds_by_file with the broader whole-file node set, which could mask a real confinement in the lines that actually changed behind an untouched sibling symbol that reaches elsewhere. Keep the precise range-based entry when one already exists. - The confinement warning is now one line per file instead of one combined message with a summed node count, matching what _render already does per file. - Regression tests for both: a rename+edit fixture (new leaf_sibling node in the shared fixture) and hook.py's render() cap behavior.
…real - The confinement check flagged a file even when its own seed IS a registered journey entry with no callers on record -- a correctly confident selection, not an unknown. Reproduced against this repo's own index: 7 of 11 "confined" files had already selected a journey at confidence 1.0. Now skipped when the file's closure intersects entry_map. - Dropped the confinement text from `warnings` entirely -- it was printed twice on a push (once via hook.py's dedicated line, once via the capped warnings loop). `closure_confined` on the result is now the sole source; both select._render and hook.render read from it independently. - Removed the dead `reached_files and` guard (closure_files can never return empty here) and consolidated the three unmapped/unmapped_files append sites behind one _mark_unmapped helper so the pairing can't drift. - Regression tests for the false-positive and the no-longer-possible double-print.
- Judge confinement per SEED, not per FILE: a file with both a registered entry (fine on its own) and an unrelated non-entry seed was having the entry silently clear the whole file, swallowing the NOTE for the seed that actually needed it. - closure_files() returns None, not a smaller set, when the closure reaches an id with no matching nodes row -- a dangling edge read identically to "resolves to no other file" and could manufacture a false confinement out of an inconsistent index. - hook.py's NOTE line is now capped (MAX_CONFINED=3) like journeys and warnings already are, so a wide rename touching many files can't print one unbroken multi-hundred-character line. - Regression tests for all three.
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.
Summary
select()seeds can resolve to real symbols whose reverse-impact closure never leaves the file the seeds started in — previously indistinguishable from a confident, correctNONE.recall_degraded, which covers unmapped seeds) reported asclosure_confinedon the result, plus a warning/NOTE line.Test plan
python3 -m pytest tests/ -q— 253 passed