Re-anchor the exemption registry to symbols and pin it there - #257
Merged
Conversation
exemptions.json anchored 44 of its 52 entries by line number. A line anchor is a pointer any edit moves: inserting a single line above a registered noqa silently un-registered a blessed exemption, and cf-exemptions then reported UNREGISTERED_SUPPRESSION against the suppression, naming the code as the culprit when the registry pointer was what rotted. Convert 41 of the 44 to the enclosing symbol, resolved with the gate's own _symbol_spans/_enclosing_symbol rather than by eye. Count-neutral: 52 entries in, 52 out, frozen_count untouched, and the only lines that differ are the 41 symbol_or_line values. Three entries stay line-pinned because a symbol anchor provably cannot identify them uniquely: git/scratch.py S105 sits at module level with no enclosing symbol, and onboard/server.py's two S101 suppressions share one enclosing method, where a symbol anchor would cover both and blanket-bless any future assert in that handler. Add tests/unit/test_exemption_anchors_are_drift_proof.py, the durable half: every entry must be symbol-anchored except an explicit map of the three stuck ones (so a re-introduced line anchor fails and a stale pin fails), every entry must resolve to exactly one live suppression (so an ambiguous anchor and a dead entry both fail), and the small resolver those checks use must agree entry-for-entry with cf_quality's own matcher wherever the kit is importable. A fourth test rods the measure itself against a synthetic module with two suppressions in one method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Retires the line-anchor class in
exemptions.jsonand pins it shut.exemptions.jsonanchored 44 of its 52 entries by line number. A line anchor is a pointer any edit moves, so inserting a single line above a registerednoqasilently un-registered a blessed exemption andcf-exemptionsthen reportedUNREGISTERED_SUPPRESSIONagainst the suppression - naming the code as the culprit when the registry pointer is what rotted. That message is convincing, and it cost three lanes an afternoon.Count-neutral. 52 entries in, 52 out,
frozen_count52, edited as surgical text (neverjson.dumps). The diff is+41/-41onexemptions.json, every changed line is asymbol_or_linevalue, and the em-dash byte count is unchanged at 66 (counted as byte sequences, notgrep -c, which counts lines). Zero source edits - no# noqawas touched.1. Converted 41 of 44. Three are stuck.
Every suppression's enclosing symbol was resolved with the gate's own
_symbol_spans/_enclosing_symbol, never by eye and never by regex. An entry was migrated only where the resulting(file, rule, symbol)triple resolves to exactly one live suppression - verified per entry, not assumed from a table.src/bonfire/git/scratch.pyS105line 55def/class, so_enclosing_symbolreturnsNone. There is no symbol to name.src/bonfire/onboard/server.pyS101line 446S101suppressions resolve toFrontDoorServer._ws_handler.src/bonfire/onboard/server.pyS101line 447Leaving these line-pinned is the finding, not the failure. For 47/48 a symbol anchor would kill one of the two entries (52 entries would stop mapping to 52 suppressions, with nothing saying so) and would turn a specific exemption into a blanket one: a future
assertdropped into_ws_handlerwould be auto-blessed with nobody approving it.What a correct fix would need - recommended, not taken. No new anchor format was invented.
assertper helper, or one combined assert), which puts them in different enclosing symbols and needs no format change at all. The alternatives are kit-side: a compound anchor (FrontDoorServer._ws_handler#2) or a per-occurrenceindexfield. Both belong tocandyfactory-quality, not to this repo, and source edits were out of scope - so neither is this lane's to take.<module>plus an index) or a code change moving the constant inside a function. Same ownership answer.2. The 7 colliding
(file, rule)pairs: 6 were safe, 1 was notThe brief's table proved
(file, rule)collisions exist; it did not prove which of them share a symbol. Measured:dispatch/security_hooks.pyC901_match_write_edit_sensitive_path/ 843_peel_one/ 895_unwrap/ 1084build_preexec_hook/ 1121build_preexec_hook._hookdispatch/security_hooks.pyPLR0915build_preexec_hook/ 1121build_preexec_hook._hook_enclosing_symboltakes the innermost span, so the nested_hooknever collapses into its parenthandlers/merge_preflight.pyBLE001MergePreflightHandler.handle/ 564MergePreflightHandler._get_baseline_failureshandlers/sage_correction_bounce.pyBLE001SageCorrectionBounceHandler.handle/ 539SageCorrectionBounceHandler._run_correction_cycleknowledge/backend.pyBLE001LanceDBBackend.query/ 117LanceDBBackend.get_by_sourceonboard/server.pyS101FrontDoorServer._ws_handler/ 447FrontDoorServer._ws_handlerscan/tech_scanner.pyC901_extract_pyproject_deps/ 245TechScanner._detect_frameworksOnly the adjacent pair collides, exactly as suspected. The other six resolve to different functions and migrated cleanly.
Correction worth carrying: the blanket-permission risk is already mechanically gated at kit altitude.
cf-exemptionsfails an entry that covers more than one live suppression (EXEMPTION_ENTRY_OVERLOADED). An ambiguous conversion therefore goes loudly red, not silently blanket. That does not make the conversion safe to do - it makes it impossible to land. Rodded below.3. The new gate:
tests/unit/test_exemption_anchors_are_drift_proof.pyThe kit's overload check covers one of three failure modes. Two are ungated: nothing stops a new line anchor being introduced, and nothing notices a dead entry that covers no suppression at all (
frozen_count == entries, so not even the ratchet-slack line prints). The new test pins all three, in this repo, on stdlib only.test_every_anchor_is_a_symbol_except_the_documented_line_pins- the set of line-anchored entries must equal_LINE_PINNEDexactly. A re-introduced line anchor fails; a pin that becomes fixable and is not removed also fails, so the allowlist cannot go stale. Carries a non-vacuity assertion that the symbol-anchored population is non-empty.test_no_exemption_entry_covers_more_than_one_suppression- every entry, either anchor form, must resolve to exactly one live suppression. Catches ambiguity and dead entries.test_the_ambiguity_measure_reports_two_for_a_deliberately_ambiguous_anchor- control rod on the measure itself: a synthetic module with twoS101in one method must resolve to[3, 4], a unique anchor to[7], a line pin to[4]. The assertion is proved able to fail, not merely observed passing.test_the_local_resolver_agrees_with_the_installed_quality_kit- round trip. The resolver used by 1-3 must agree entry-for-entry withcf_quality.exemptions._scan_src/_matcheswherever the kit is importable, so the mirror cannot drift from the gate it mirrors.Proof it catches a deliberately ambiguous anchor. Point entry 47 at the shared symbol, then run the new test:
Proof it catches a re-introduced line anchor. Regress
knowledge/backend.pyBLE001back to"78":Proof the kit's own overload check is real, not merely present in its source:
4. Control rods, both halves, verbatim
Drift-proof half - A/B/A on the named failure. The same 30 inserted lines above the suppressions in
knowledge/backend.py,scan/tech_scanner.py,dispatch/security_hooks.pyandhandlers/merge_preflight.py, applied to two trees:Same insertion, same gate, same command:
mainreports 14 phantom culprits, this branch reports none. That is the defect, measured.Not-defanged half. A fresh, never-registered
# noqa: BLE001injected intoknowledge/chunker.pyon this branch:The gate still bites. Every rod ran on scratch copies; the worktree was never dirtied.
5. complexipy-snapshot.json
ce1dbdc0ff983e828e4a9ba69998fe67- byte-identical toorigin/main(git show origin/main:complexipy-snapshot.json | md5summatches), 20 entries, non-empty. Verified before the gate runs, after each of the threecf-gateruns, and as the last action.6. Gate verdict - the delta is zero
cf-gaterun from an environment withbonfireand the kit's[dev]extra installed, on this branch and on a fresh export of bareorigin/mainprepared identically:The red-gate detail block is byte-identical between the two (
diffclean):MYPY_NEW_ERRORS(the local-kit artefact) andCOMPLEXIPY_FAILED. Nine gates green on both, includingcf-exemptions,cf-file-budgetandpytest. Nothing in this PR is red.Local suite: 5777 passed, 4 skipped, 37 xfailed, 20 xpassed.
ruff checkandruff format --checkclean acrosssrc/andtests/.7. One budget raise, surfaced not taken silently
tests/unit73777 -> 74023 (+246), the exact measured line count of the new test file, recorded inpackage_raiseswith its reasoning. Explicitly rejected: parking the file at unbudgetedtests/root (two earlier raises in that same file already rejected the unbudgeted tree as dodging the ratchet, and the same answer applies here); apurposeentry; dropping the kit-agreement test to save ~25 lines. The file was tightened from 261 to 246 lines before the number was taken. Nothing was added to or removed fromexemptions.json.8. What I did NOT prove
EXEMPTION_ENTRY_OVERLOADEDwas proved to fire in the locally installed kit (candyfactory-qualityat74b346c, editable install). CI pinsquality-gate.yml@adaa4ebf...; that SHA's copy ofexemptions.pywas not read. "The ambiguity gate is enforced in CI" is inferred from the local kit, not measured against the pin.cf_qualityis not abonfiredev dependency, sotest_the_local_resolver_agrees_with_the_installed_quality_kitpasses where the kit is installed and skips under a plainpip install -e ".[dev]". Tests 1-3 always run. The round trip is a local/kit-env check, not a CI-enforced one.deforclassun-registers its exemptions exactly as a line shift used to, and the gate will again name the suppression as the culprit. This PR does not close that; it removes the far more frequent trigger. Worth a follow-up note in the gate's own error text.# noqaedits, so nothing here can change what ruff suppresses - but neither was it independently re-verified that all 52 suppressions are still needed.origin/maindelta method.Out of scope, one sentence:
cf-file-budgetreports two shrink opportunities untouched by this PR -tests/unit/test_engine_gates.py621 -> 615 andtests/unit/test_engine_pipeline.py1895 -> 1894 - which would hand the next lane 7 lines of headroom back.