fix(deps): a floor report states its role and scope; detect a production bound the CI plane cannot keep (#17558) by mrveiss · Pull Request #17610 · mrveiss/AutoBot-AI

mrveiss · 2026-09-27T09:18:42Z

Thinking Path

Three readers misread a dependency-floor block in one day — twice as a build
failure it was not, once as a clean environment it was not. Each reading was of
correct data whose role and scope were unstated. One property, not three bugs.

The most compact proof that the return type was the problem rather than the
wording: I quoted the number wrong myself, in the issue I filed about it. I
wrote 22 of 216 NOT satisfied. It is 22 of 198 compared — 216 declarations
read, 18 never installed and so never compared. The output had no way to express
the difference between declared, compared and installed.

The second commit then found the same family one layer out: #17557 was
invisible to check_requirements_ci_drift.py because it compares packages
declared in both planes, and opentelemetry-api was pinned in one and merely
installed in the other.

Refs #17558

What Changed

Commit 1 — the report states its role, its scope, and what it could not compare.

audit() returns a FloorAudit instead of (shortfalls, declarations_read); the
second element was printed as a comparison count and is not one. Every block now
says [GATE] or report only (informational; nothing here fails this run), and
prints the declaration roots. repo_tests/dependency_floor_banner.py is the
report half and printed a red environment is BELOW the declared dependency floors header in the same shape as the gating check; it now says what it is.
Under --venv against every root, an ADVISORY names the fix.

The reporting half moved to pipeline-scripts/dependency_floor_report.py because
the file reached 639 lines against the 600 ceiling. The ceiling was not
raised, and the seam is genuine: nothing there measures anything and it imports
nothing from the checker — which it must not, because the banner loads the
checker via spec_from_file_location, where a sibling import would not resolve.

Commit 2 — a production upper bound the CI plane cannot keep.

tools/lint/check_ci_plane_unenforced_pins.py: transitive closure of the CI
plane ∩ names the production plane bounds above, minus what the CI plane
declares. Two refinements, both load-bearing — a bare >= is safe under an
unbounded resolve, so the trigger is an upper bound the CI plane does not
restate; and a requirer that caps it itself makes the agreement structural,
so langgraph is correctly not reported.

A separate module, deliberately: the drift checker is purely static, and folding
in a metadata-dependent question would make a static guard environment-dependent
— the conflation this issue exists to remove. It imports that checker's parsers,
so the planes are still parsed in one place.

Verification

Same deployed venv, minutes apart, each verdict now stating its scope:

--venv …/autobot-backend/venv
  [report only] 22 of 198 compared versions below floor; 216 declarations read;
    18 declared but NOT INSTALLED, therefore not compared
    roots: <4 roots>
    ADVISORY: compared against EVERY declaration root …

--venv … --roots autobot-backend/requirements.txt
  [report only] 94 of 94 declarations compared … all satisfied

Both were always true. Neither line used to say which question it answered.

The contrast pair is free. The four OpenTelemetry packages are silent now and
would have fired before #17502 declared them. The check reproduces #17557 when
the fix is absent and says nothing when present.

Two findings the work produced about itself:

protobuf is a live unenforced pair and my designing measurement said it
was not. That sweep recorded only the first requirer per package;
protobuf's first-reached requirer caps it at <8.0, its fourth —
onnxruntime — requires protobuf>=4.25.8 unbounded. Production caps
<8.0.0, a bound re-derived under tech-debt(deps): the protobuf constraint and its inline rationale contradict each other in both requirements files #15070 with onnxruntime absent from that
analysis. Filed as deps(protobuf): the CI plane never declares protobuf, so it will resolve past production's re-derived <8.0.0 cap #17582, baselined not fixed: the fix moves what CI
installs.
The guard committed its own defect and the pre-push hook caught it. Keyed
on package names alone, an interpreter lacking a requirer saw no pair and the
bidirectional contract fired a confident "torch is no longer an unenforced
pair". BASELINE now maps each package to its unbounded requirer, so the check
asks is that requirer installed here? before calling an entry stale.
Evaluability is per entry, not per plane — a coverage threshold was the
first attempt and the wrong instrument, since the interpreter that broke it
held most of the plane and lacked the one requirer that mattered.

Registering the new checker with the reach meta-guard found a defect there
too. _load_module never set sys.modules, so @dataclass annotation
resolution failed — that loader could not read any guarded checker defining a
dataclass. Fixed, plus a sys.modules.pop on failure. Reach audit covers 19
guarded paths, up from 15.

Counts: 117 + 19 tests pass; four mutations each fail a test on the reporter,
three on the guard. Verified in the full venv (2 pairs, 0 not evaluated, exit 0)
and python3.10 (1 pair, 1 not evaluated with the requirer named, exit 0).
Local runs on this session's venv; CI is the authority on the declared set.

Model Used

Claude Opus 5 (1M context) — claude-opus-5[1m]

Summary by CodeRabbit

New Features
- Added a dependency-pin audit that identifies production version caps not enforced by reachable CI dependencies. Audit mode reports new and outdated baseline findings.
- Dependency-floor reports now show how many declarations were compared, identify packages that are not installed, and provide guidance tailored to the audit environment.
Bug Fixes
- Dependency-floor summaries now use an informational banner for shortfalls rather than presenting them as a red failure warning.
- Reports now distinguish declared dependencies from those actually compared, avoiding misleading comparison counts.

…ld not compare (#17558) Three readers misread a dependency-floor block in one day -- twice as a build failure it was not, once as a clean environment it was not. Every reading was of correct data whose ROLE and SCOPE were unstated. That is one property, not three bugs, and it is fixed once here. WHAT A NUMBER NOW CARRIES. `audit()` returns a `FloorAudit` instead of `(shortfalls, declarations_read)`, because the second element was printed as though it were the number of comparisons and it is not. A package declared but not installed is dropped by `installed_versions` -- compared against nothing -- so "128 declarations checked, all satisfied" could be, and was, true of an environment holding 86 of them. Measured on one box: 128 declared, 42 never compared. A pass now reports comparisons and names what it could not compare. ROLE. Every block says whether it is a `[GATE]` or `report only (informational; nothing here fails this run)`. repo_tests/dependency_floor_banner.py is the report half and printed a RED "environment is BELOW the declared dependency floors" header in the same shape as the gating check; it now says what it is and prints yellow. It cannot fail a run and never could. SCOPE. The declaration roots are printed. The same deployed venv reports `22 of 216 NOT satisfied` against every root and `94 ... all satisfied` against the one that builds it -- both true, answering different questions, and neither line said which. With `--venv` against the union the report carries an ADVISORY saying so and naming the fix, because a shortfall from a plane that does not build the environment is a false positive, not a finding. `--all` lists the not-compared names too: a flag promising every shortfall while hiding the packages no shortfall could be computed for is the same omission one level down. SIZE. The change took check_dependency_floors.py to 639 lines against a ceiling of 600, so the reporting half moves to pipeline-scripts/dependency_floor_report.py (550 and 147). The ceiling is not raised and the seam is real: nothing in the new module measures anything and it imports nothing from the checker, so the dependency runs one way -- required, because the banner loads the checker with spec_from_file_location, where a sibling import would not resolve. `FloorAudit` carries `roots_are_the_union` rather than re-deriving it from `DECLARATION_ROOTS` for the same reason. `_print_report` is extracted from `main` for the #620 length gate, which caught it at 66 body lines against a threshold of 65. Verified against a deployed venv as well as fixtures. Four mutations each fail a test: counting declarations instead of comparisons, dropping the roots line, labelling every block a gate, and hiding the not-installed count. 117 tests pass across the checker, the strict gate and the ci-drift suites. Items 2 and 4 of #17558 -- one-plane-only drift detection, and the sweep for CI-plane packages with unbounded transitive deps the root plane pins -- are not in this commit.

…17558, #17557) Items 2 and 4. `check_requirements_ci_drift.py` compares packages declared in BOTH planes, so #17557 was invisible to it: `opentelemetry-api` was pinned in one plane and merely INSTALLED in the other, dragged unbounded by `chromadb==1.5.9`. No pair to compare, so the check was correctly silent while the planes diverged, and the coincidence ended when 1.45.0 shipped -- eight unrelated PRs red, none of which had touched a requirements file. The new check enumerates the shape instead of waiting for it: transitive closure of the CI plane, intersected with the names the production plane bounds ABOVE, minus what the CI plane declares. TWO REFINEMENTS, BOTH LOAD-BEARING. Either alone gives a wrong answer. A bare `>=` floor is safe under an unbounded CI resolve, because newest satisfies it -- so the trigger is an upper bound the CI plane does not restate. Filtering on `==` alone finds one of three candidates here. A requirer that bounds the package above ITSELF makes the agreement structural. `langchain` requires `langgraph<1.3.0` where production says `<2.0.0`: stricter, so the resolution cannot escape. Reporting those is the false-positive noise that makes a check ignored, which is #17558's own subject. THE CONTRAST PAIR IS FREE. The four OpenTelemetry packages are silent now and would have fired before #17502 declared them. The check reproduces #17557 when the fix is absent and says nothing when it is present. IT FOUND ONE THING THE MEASUREMENT THAT DESIGNED IT GOT WRONG. That sweep recorded only the FIRST requirer per package and reported `protobuf` as structurally enforced. `protobuf`'s first-reached requirer caps it at `<8.0`; its fourth, `onnxruntime`, requires `protobuf>=4.25.8` with no upper bound at all. Production caps `<8.0.0`, so CI takes 8.0 the day it ships. A first-match sweep reports a property of its iteration order as a property of the world; the closure docstring says it records every requirer and names this as the reason. So BASELINE is two entries. `protobuf` is filed as #17582 rather than left as a footnote -- the fix is #17502's applied again, but it moves what CI installs, and that cap was already got wrong once and re-derived under #15070 with `onnxruntime` absent from the analysis. It deserves its own review. A SEPARATE MODULE, deliberately, against the same-file instinct: the drift checker is purely static and reads requirement files only. This must read installed distributions to know what the CI plane drags in, and folding a metadata-dependent question into a static guard would make the static one environment-dependent -- the conflation #17558 exists to remove. It imports the drift checker's parsers, so the planes are still parsed in one place, and it names the interpreter its closure came from. rule is stated as data. `GUARD_INPUT_PATHS` is declared and the checker is registered with the reach meta-guard, which #17542 exists because this one's sibling was not. 19 tests, none of which read the real environment: the closure is injected so the rule is stated as data. REGISTERED WITH THE REACH META-GUARD, and that registration found a defect in the meta-guard. `GUARD_INPUT_PATHS` alone does nothing -- #17542 exists because this checker's sibling declared it and was never added to `_GUARDED_CHECKERS`. Adding it made `check_code_quality_guard_reach.py` fail with `AttributeError: 'NoneType' object has no attribute '__dict__'`: its `_load_module` never registered the module in `sys.modules`, and `@dataclass` resolves annotations through `sys.modules[cls.__module__]`. So that loader could not read ANY guarded checker defining a dataclass, and the error named neither the cause nor the file. `repo_tests/dependency_floor_banner.py` documents the same trap for the same reason; this is that fix one guard over, plus a `sys.modules.pop` on failure so a half-executed module is never left registered for the next load to reuse. The reach audit now covers 19 guarded paths, up from 15. THE GUARD COMMITTED ITS OWN DEFECT AND THE PRE-PUSH HOOK CAUGHT IT. The first version keyed BASELINE on package names alone, so an interpreter lacking a requirer saw no pair and the bidirectional contract fired a confident "torch is no longer an unenforced pair". The hook runs a leaner interpreter than this was written against (#16715): torchmetrics absent, verdict inverted. The contract was right about its data; the data was the problem -- *did not look* rendered as *nothing found*, inside the change built to stop that. Two fixes. BASELINE now maps each package to the unbounded requirer that makes it a pair, so the check asks "is that requirer installed here?" before calling an entry stale. And evaluability is per ENTRY, not per plane: a coverage threshold was the first attempt and the wrong instrument, because the interpreter that broke it held most of the CI plane and still lacked the one requirer that mattered. audit() returns (problems, notes, pairs). A pair this interpreter cannot judge is neither a failure nor a pass: gating it would block every push from a leaner environment, dropping it would be the silence this guard exists to prevent. Same gate-versus-report split as the floor reporter in the commit below. Verified in both: the full venv reports 2 known pairs and 0 not evaluated, exit 0; python3.10 reports 1 known pair, 1 not evaluated with the requirer named, exit 0.

coderabbitai · 2026-09-27T09:18:52Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9a47dca6-09ab-4c27-9270-b6a3caaa05e5

📥 Commits

Reviewing files that changed from the base of the PR and between 211d3a7 and 1d592de.

📒 Files selected for processing (4)

pipeline-scripts/check_dependency_floors.py
pipeline-scripts/check_dependency_floors_test.py
pipeline-scripts/dependency_floor_report.py
pipeline-scripts/dependency_floor_report_test.py

📝 Walkthrough

Walkthrough

The pull request changes dependency floor audits to return structured results and share report rendering across command-line and pytest output. It also adds a checker for production upper bounds that are not enforced by dependencies reachable from the CI-installed packages.

Changes

Dependency floor reports

Layer / File(s)	Summary
Audit report contract and rendering `pipeline-scripts/dependency_floor_report.py`, `pipeline-scripts/dependency_floor_report_test.py`	Adds `FloorAudit` and a shared renderer for gating and informational reports. Tests cover report counts, scope, missing packages, detail limits and environment guidance.
Structured audit and command-line reporting `pipeline-scripts/check_dependency_floors.py`, `pipeline-scripts/check_dependency_floors_test.py`	Changes `audit` to return `FloorAudit` with declaration and comparison counts, shortfalls, absent package names, roots and environment data. The command-line path uses the shared renderer and bases strict-mode status on non-exempt shortfalls.
Pytest banner reporting `repo_tests/dependency_floor_banner.py`, `repo_tests/dependency_floor_banner_test.py`	Updates the pytest terminal hook to use the audit result and print an informational banner when shortfalls exist. Tests use `FloorAudit` results.

CI unenforced dependency pins

Layer / File(s)	Summary
Dependency closure and unenforced-pair detection `tools/lint/check_ci_plane_unenforced_pins.py`, `repo_tests/ci_plane_unenforced_pins_17558_test.py`	Adds requirement parsing, upper-bound recognition, CI dependency-closure traversal and detection of production upper bounds that lack enforcement. Tests cover bounds, markers, optional requirements and transitive requirers.
Baseline audit and checker integration `tools/lint/check_ci_plane_unenforced_pins.py`, `tools/lint/check_code_quality_guard_reach.py`, `.github/workflows/code-quality.yml`, `repo_tests/ci_plane_unenforced_pins_17558_test.py`	Classifies new and stale baseline pairs as problems and unevaluated entries as notes. Adds the audit command-line option, guarded-checker integration and the `packaging` installation used by lint checkers.

Priority: ⬇️ Low

Merge Risk: 🟡 Moderate · up to 211d3

Floor reports can misstate what was compared or found below its floor. Correct those counts and labels before merging so the reports convey their intended scope accurately.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 211d3

The new dependency-bound check is not invoked by the code-quality workflow, so the intended protection is not active there. The existing check remains in place, but it does not cover the same case.

Retained concerns

Medium · security · observed: The new production-bound checker is registered for input-path coverage but is not invoked by the code-quality workflow. Thus its newly detected pairs cannot fail that workflow. If invoked in a lean interpreter, its installed-metadata scope can also leave pairs unevaluated without failing.

Security review details

Security Blast Radius

inferred — The demonstrated control gap is in repository-wide CI dependency checking, not a changed production install or public service entrypoint. Its effect is a possible missed dependency-policy signal; no production exploit or privilege gain is established.

Security Findings and Attack Paths

inferred — A production upper bound absent from CI declarations could remain undetected by the proposed new workflow control because that workflow does not run the checker. This is a control-coverage path, not a verified vulnerability in a particular dependency.

Trust Boundaries and Controls

observed — The checker treats declarations and installed package metadata as distinct inputs. It excludes optional-extra requirements and records all reachable requirers, but cannot traverse metadata for a distribution absent from its interpreter.

Resilience and Maintainability Implications

observed — Missing baseline requirers are reported as NOT EVALUATED rather than silently treated as clean, but those notes do not fail the audit. A lean interpreter therefore cannot establish complete baseline coverage.

Hardening Proposals

proposed — Invoke the new checker with --audit in a workflow environment containing the intended CI dependency closure, and make unevaluated coverage explicit in the gate's policy.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 67 functions across 11 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarises both primary changes: clarifying floor-report role and scope, and detecting production bounds that the CI plane cannot enforce.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 64.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 67 functions across 11 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-09-27T10:04:40Z

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

…ort-roots

…#17558) Both floors sat 201 over an allowance of 200 on the merge of this branch with main, so `reach_declarations_test.py` failed twice for the same reason from two angles. Accumulation, and this branch is the module that crossed the line. The allowance absorbed 200 test modules added across every session since the last pin; this branch adds exactly one, `repo_tests/ci_plane_unenforced_pins_17558_test.py`, and 201 - 1 = 200 is precisely the allowance. Without it the gap passes. So the crossing is shared and the last step is mine, which is what the allowance is for. Re-pinned to the measured live populations -- 2540 -> 2741 and 2527 -> 2728 -- rather than widening `growth`. That tolerance is what let two hundred modules land without a re-pin; raising it makes the next crossing later and larger, which is the opposite of what a ratchet is for. Not reproducible before merging main: this branch was 4 commits behind and measured 196, under the allowance. CI evaluates the merge, so the numbers only matched after `git merge origin/main` -- 2741 and 2728, the exact figures CI reported. Worth stating because "cannot reproduce locally" was the first reading and it was an artefact of the branch's base, not a flake. Main had not re-pinned: `origin/main` still declared 2540 and 2527 when checked, so this was nobody's housekeeping yet.

…into issue-17558-floor-report-roots

mrveiss · 2026-09-27T11:00:46Z

Status note, not a finding: every workflow on `5353afa5e` is `cancelled` — all eight at 10:40:33, with no superseding run.

The three red contexts are the refusal, not a defect:

verify-generated-types · Report generated-types verdict
  CHANGES_RESULT: cancelled
  RUN_RESULT: cancelled
  ::error::path detection cancelled — cannot tell whether generated types need checking

smoke-test and migration-matrix failed in the same second with the same shape. That step is behaving correctly: a cancelled detection is not a success, and reporting it as one is the failure mode these gates exist to prevent. There is nothing in the diff to fix.

The two reach floors that failed on 08025b369 earlier are already ratcheted on this head — blanket-skip-test-module-sweep 2540 → 2741 and collectable-modules-inert-on-import 2527 → 2728, both re-measured rather than carried across.

Not re-running yet: 38 runs are in flight repo-wide and this PR is not the one landing. Re-run at the next sweep.

… without noticing (#17571) `prepush_frontend_projects_17574_test.py` feeds sample paths to the pre-push hook's filter. They must NOT resolve: `python_filter_covers_its_guards_test` records any quoted path that resolves as a tree this guard depends on, and the remedy for that is an entry in a shrink-only record which nothing can later remove. The docstring already carried two constraints pulling against each other. A third arrived from the tree rather than from a guard. The paths were `Chat.vue`, `useThing.ts`, `RolesView.test.ts`, `Button.vue` -- ordinary names in ordinary directories. #17616 created `autobot-slm-frontend/src/views/RolesView.test.ts` and `.../composables/usePerformanceMonitoring.test.ts` for entirely unrelated reasons, and two fixtures became real. That breaks this guard AND pushes the uncovered-reads record from 45 to 47. MAIN IS CARRYING THIS BREAK RIGHT NOW AND ITS BADGE IS GREEN. #17616 changed only `autobot-slm-frontend/**`, which the python-paths filter does not cover, so `python-suite` was SKIPPED on `bf6ee5583` -- every shard, reported as `skipped`, run conclusion `success`. The bypass that let it through is the exact one `python_filter_covers_its_guards_test` exists to name: "a change confined to one takes the required-context shim's green while the guard never runs." The guard predicted the hole, the change went through the hole, and what it landed on was the guard. Nothing was wrong with either; the two never met, because the suite holding one of them did not run. Fixed here rather than in a new PR because this PR was already red on it and unblocking base is worth more than tidy attribution. - Every fixture path now carries the reserved `__prepush_fixture__` segment. Project prefix and extension are unchanged, because those are what the hook's filter routes on; only the basename moves. - `test_every_fixture_path_is_reserved_by_name` makes not-resolving a property of the NAME. `test_no_fixture_path_names_a_real_file` checks the tree as it stands, which is only ever a statement about the present -- it passed for months and then someone added a file. The reserved segment is independent of what anyone adds next. - The composable-sibling test builds its sibling under `tmp_path`, so its name tracks the input for matching only; nothing there touches the real tree. Also: two reach floors, MEASURED in this tree rather than copied from #17610 which ratchets the same two: blanket-skip-test-module-sweep 2540 -> 2741 test modules collectable-modules-inert-on-import 2527 -> 2728 matching modules Ratchet baselines are hub files and exempt from one-PR-one-owner; whichever of the two PRs merges second resolves a one-line conflict. Verified: 318 passed, 0 failed across prepush_frontend_projects_17574_test.py, python_filter_covers_its_guards_test.py and reach_declarations_test.py. Refs #17571

coderabbitai

Actionable comments posted: 4

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 04673580-0b07-4395-84ec-7530d370fd74

📥 Commits

Reviewing files that changed from the base of the PR and between 3374346 and be3964b.

📒 Files selected for processing (10)

pipeline-scripts/check_dependency_floors.py
pipeline-scripts/check_dependency_floors_test.py
pipeline-scripts/dependency_floor_report.py
repo_tests/blanket_skips_carry_a_reason_test.py
repo_tests/ci_plane_unenforced_pins_17558_test.py
repo_tests/collected_modules_inert_on_import_test.py
repo_tests/dependency_floor_banner.py
repo_tests/dependency_floor_banner_test.py
tools/lint/check_ci_plane_unenforced_pins.py
tools/lint/check_code_quality_guard_reach.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

…s, parse markers (#17558) All four are the branch's own family: a value accurate about something other than the question asked. COUNT DECLARATIONS, NOT NAMES. `absent` is de-duplicated, so `len(declarations) - len(absent)` credited the extra declarations of an uninstalled package as compared. A package declared in two service files, or reached twice through `-r`, made the pass line report "N of M compared" with the wrong N -- this issue's own defect, one layer inside the fix for it. ABSENCES ARE NOT COMPARISONS. With `--require-present` an absent declaration becomes an ABSENT shortfall while `compared` excludes it, so the headline could read "12 of 5". Absences already have their own line; they no longer sit in the numerator of a comparison count. A MARKER IS NOT A SPECIFIER. `bounds_above` scanned the whole requirement suffix, so `widget>=1.0; python_version < "3.10"` looked capped because of the `<` in its MARKER -- turning an unbounded requirer into a structurally bounded one and HIDING a real pair. A false negative in the direction that matters for a guard whose job is finding pairs nothing bounds. `specifier_of` parses and returns only the specifier; an unparseable requirement drops everything after `;` rather than scanning it, under-claiming a bound instead of inventing one. `extra=="x"` IS AN EXTRA. The optional-dependency filter was a substring check for `"extra =="`, which missed valid metadata written without spaces; a retained optional requirement invents a pair nothing installs. `is_optional` now evaluates the marker with an empty extra. `packaging` added to the code-quality install step: that job installs its tools explicitly and never the application's dependencies. Both checkers degrade rather than crash without it, and degrading silently is what this branch removes. The additions took check_dependency_floors_test.py to 614 against the 600 ceiling, so the render-facing tests move to pipeline-scripts/dependency_floor_report_test.py (542 and 119). The ceiling is not raised and the seam follows the module split they test. Six tests added; four mutations each fail one -- counting per unique name, returning ABSENT shortfalls to the numerator, scanning the whole suffix, and restoring the substring extras check.

…into issue-17558-floor-report-roots

…st (#17558) `dependency_floor_report_test.py` installed `check_dependency_floors` in `sys.modules` so `exec_module` could run, and never removed it. The key is needed only WHILE the module body executes -- a dataclass defined in it resolves its own module by name. Left installed, a `pipeline-scripts/` module sits in the `sys.modules` of every test that runs afterwards. The leak guard caught it: LEAK: pipeline-scripts/dependency_floor_report_test.py leaves 1 sys.modules key(s) installed outside pipeline-scripts/ Install and remove in the same try/finally, which is what the guard's own message asks for. The `checker` reference the tests use stays valid; only the global registry entry goes. Worth noting where this came from: the file is new in the previous commit, and that commit was written and never pushed. The suite it belongs to reported "112 passed" while exiting non-zero, because the leak guard reports after the summary line -- reading the count and not the exit code would have shipped it. Refs #17558

mrveiss · 2026-09-27T14:08:01Z

All four threads are addressed and pushed — f85ab2b8d9. They resolved as outdated before I replied, so recording the evidence here rather than losing it.

All four are this branch's own family: a value accurate about something other than the question asked.

Thread	Finding	Fix
`check_dependency_floors.py:445`	`absent` is de-duplicated, so `len(declarations) - len(absent)` credited the extra declarations of an uninstalled package as compared. A package in two service files, or reached twice via `-r`, made the pass line report "N of M compared" with the wrong N	`compared = sum(1 for d in declarations if d.name in installed)`
`dependency_floor_report.py:109`	With `--require-present` an absent declaration becomes an ABSENT shortfall while `compared` excludes it — the headline could read "12 of 5"	absences leave the numerator; they already have their own line
`check_ci_plane_unenforced_pins.py:163`	`bounds_above` scanned the whole suffix, so `widget>=1.0; python_version < "3.10"` looked capped because of the `<` in its marker — an unbounded requirer read as bounded, hiding a real pair	`specifier_of` parses and returns only the specifier; an unparseable requirement drops everything after `;` rather than scanning it, under-claiming a bound instead of inventing one
`check_ci_plane_unenforced_pins.py:185`	the optional-dependency filter was a substring check for `"extra =="`, missing valid metadata written without spaces; a retained optional requirement invents a pair nothing installs	`is_optional` evaluates the marker with an empty extra

Note the direction on the third: a guard whose job is finding unbounded pairs failed toward silence, which is the direction that does not announce itself.

Also in that commit: packaging added to the code-quality install step. That job installs its tools explicitly and never the application's dependencies; both checkers degrade rather than crash without it, and degrading silently is what this branch exists to remove.

Verification. Six tests added, and four mutations each fail exactly one — counting per unique name, returning ABSENT shortfalls to the numerator, scanning the whole suffix, restoring the substring extras check. Locally 112 passed, 0 failed.

One more defect, found while verifying and fixed in b18e7a8a2c. The new dependency_floor_report_test.py installed check_dependency_floors in sys.modules for exec_module and never removed it, leaking a pipeline-scripts/ module into every test that ran afterwards:

LEAK: pipeline-scripts/dependency_floor_report_test.py leaves 1 sys.modules key(s)
      installed outside pipeline-scripts/

Install and remove in the same try/finally. Worth naming how it nearly shipped: the run printed 112 passed and exited non-zero, because the leak guard reports after pytest's summary line. Reading the count instead of the exit code would have missed it — which is the same substitution this whole branch is about.

On the earlier red: it was CHANGES_RESULT: cancelled, not a test failure. The reporting step refusing to call a cancelled detection a success is correct behaviour.

The file split (check_dependency_floors_test.py hit 614 against the 600 ceiling) follows the module split it tests. The ceiling was not raised.

Brings in the two commits this branch's CI failures were caused by, neither of them this branch's work: the fragmentation-ratchet fix (#17634, which reduced the CSS rather than raising the pin) and the fixture-collision repair (#17615, renaming two literal paths that had become real files). Conflict on both reach floors, resolved to origin/main's newer pins -- 2741 -> 2743 and 2728 -> 2730. Mine were measured before two more test modules landed and are superseded, not competing. Verified against the merged tree rather than assumed: live 2745 and 2732, gap 2 on each against an allowance of 200. Merged rather than rebased on purpose. This branch carries commits pushed by another session; a rebase rewrites them under new SHAs and needs a force-push, and --force-with-lease does not protect a shared branch because the lease compares against a shared remote-tracking ref. A merge reaches the same base without rewriting anyone's history.

coderabbitai

Actionable comments posted: 2

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 36aa86f7-872b-4639-a74a-7865851c4818

📥 Commits

Reviewing files that changed from the base of the PR and between be3964b and 211d3a7.

📒 Files selected for processing (9)

.github/workflows/code-quality.yml
pipeline-scripts/check_dependency_floors.py
pipeline-scripts/check_dependency_floors_test.py
pipeline-scripts/dependency_floor_report.py
pipeline-scripts/dependency_floor_report_test.py
repo_tests/ci_plane_unenforced_pins_17558_test.py
repo_tests/dependency_floor_banner_test.py
tools/lint/check_ci_plane_unenforced_pins.py
tools/lint/check_code_quality_guard_reach.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

…17558) Two review findings on 211d3a7, both the defect this change exists to remove, found inside the change itself. UNREADABLE IS NOT A COMPARISON. `UNREADABLE` is a VALUE in `installed`, not an absence, so `name in installed` counted an installed-but-unreadable package as compared when nothing had been compared at all. `compared` now requires the installed version to be readable. UNREADABLE IS NOT BELOW FLOOR EITHER. `shortfalls()` emits `Shortfall(d, UNREADABLE)`, and the headline numerator excluded only ABSENT -- so an unverifiable version was reported as "below its declared floor", a verdict no measurement supports. It is now excluded from that numerator and reported as unverified on its own line. *Did not measure* had been rendering as *measured and found wanting*, which is the sentence this PR is about. THE ABSENCE COUNT NAMED THE WRONG UNIT. `not_installed` is de-duplicated by package name, so two files declaring one absent package rendered as "1 declared but NOT INSTALLED" while two declarations went uncompared. The line now carries both units, because the complement of `compared` must be countable in the same unit as `compared` or the two numbers cannot be read together. Three tests added, three mutations each fail one. One existing test asserted the old wording and is updated with the reason recorded rather than silently repointed.

…r read (#17571, #14785) (#17608) * fix(canonical): the audit harness reported clean over a tree it never read (#17571, #14785) `scripts/canonical_check.mjs --all` printed `0 violations` and exited 0 while the same rule, handed the same file directly, found one. The flag satisfied the "pass --files or --all" check and then filtered `args.files`, which `--all` never populated. The walk was marked "a Wave 3 task", so the mode existed, reported success, and had never opened anything. That is the defect the canonical rules exist to catch, inside the tool meant to catch it: the rule existed and the tree was never asked. Four mechanisms were missing, all verified by running it: --all now walks each rule's TARGETS. 51 violations where it reported 0. TARGETS is honoured. runRules ran every rule against every file, so the declaration was decorative: a rule scoped to one tree silently inspected another. A rule may also declare EXTENSIONS, which is what lets a CSS rule exist without the console.log rule scanning stylesheets. A TARGETS entry naming a missing path is reported and fails the run. It previously made its rule inspect nothing while reporting the same clean result as a rule that found nothing. An empty file set refuses rather than printing a clean run. Adds the first design-system rule the harness has carried: ds-unloaded-authority blocks on a stylesheet that claims to be the canonical token source and is imported by nothing. It fires on assets/tokens.css, which opens "Canonical CSS Design Tokens", declares no tokens at all, and appears in no import statement. Severity `block`, so the run exits 1 -- the shipped smoke rule is `warn`, and before this nothing could fail. Three bugs of my own on the way, each the shape this harness is for: The rule first needed repo context passed in; check() takes one argument, so it returned [] forever -- dead on arrival. Its import-graph needle was an unanchored substring, so looking for `tokens.css` matched `design-tokens.css`, which IS imported. The unloaded file read as loaded, and telling those two names apart is the rule's entire job. Anchored on a path boundary. I labelled it SEVERITY "error". makeDiagnostic rejects anything outside [block, warn, audit], so that crashes the run rather than passing quietly -- which also corrects my earlier claim on #17571 that "error does not fail". A rule cannot opt into failing by inventing a level; it says block or it is advisory. Nine tests added, covering the walk, TARGETS and EXTENSIONS scoping, an unreachable target, a prefix that is not a tree match (`autobot-frontend/srcx`), and that a block diagnostic exits 1. 14 pass, up from 5. The CI workflow still has no failing step, so this is enforceable rather than enforced; that is the remaining half of #17571. * feat(canonical): wire the frontend audit into CI and let a blocking rule fail it (#17571, #14785) The audit generated, uploaded and summarised, and had no failing step at all -- so a `block` diagnostic landed in an artifact nobody opened. A rule that cannot fail anything is a report, and that is precisely why the design system has been advisory. The frontend audit is now actually run. Its step previously wrote a placeholder saying `--all` was unimplemented, which was true: the flag was accepted and walked nothing, reporting a clean tree it had never opened. The exit code is captured rather than allowed to abort the generate step, and the gate runs AFTER the upload. Under `bash -e` a blocking diagnostic would otherwise kill the job before the artifact and job summary existed -- publishing nothing is how people stop reading a gate. Only `block` fails. The vocabulary is block | warn | audit and makeDiagnostic rejects anything else, so a rule cannot opt into failing by inventing a level. The 50 existing console.log findings are `warn` and do not fail. Removes `autobot-frontend/src/assets/tokens.css`, the one blocking violation, so this gate lands green instead of reddening main on arrival. Deleting it loses nothing, and that was measured rather than assumed: 310 lines, ZERO non-comment lines, zero declarations, and no import statement anywhere names it. Of the 128 token names it mentions in prose, 113 are declared in loaded files and the other 15 are wildcard prefixes (`--color-`, `--spacing-`), not names. It opened "Canonical CSS Design Tokens ... serves as the primary reference", so the file asserting authority was the one nothing loaded. The rule's tests moved off the live tree onto fixtures in the same change. Asserting "the repo contains a blocking violation" would have passed only until the backlog drained and then failed for the best possible reason -- RATCHET_BASELINES.md names this, and removing the single instance is what surfaced it. Three fixtures now: a claiming-and-unimported stylesheet blocks, an ordinary one does not, and the waiver is honoured. One live assertion remains, stated as the outcome: the tree has no blocking violations. Verified: 17 harness tests pass, up from 5; `--all` exits 0 with 50 warn diagnostics; the workflow parses. * fix(canonical): refuse a run that inspects nothing, and resolve import paths (#17571, #14785) Three review findings, all correct, and all the same class this PR is about -- a check reporting clean over something it did not examine. 1. A run with files but no COVERAGE exited 0. `--files` naming only paths outside every rule's TARGETS, or with an extension no rule accepts, left every rule skipped and still reported success. Having files is not having coverage. It now exits 2 and says how many files were given and that none were in scope. That is the `--all` bug one level in, and I had added the empty-list refusal without noticing the covered-but-skipped case. 2. Read failures became a clean audit. Both walkers caught and continued, so an unreadable directory silently shrank the sweep while the run still printed "0 violations". Fixing that surfaced a distinction I had collapsed: an ABSENT tree is not an UNREADABLE one. ENOENT on a top-level target means the tree does not exist, which `unreachableTargets` already reports, so continuing is not a silent loss. Anything else -- a permission error, or any failure on a directory the walk itself just discovered -- is a directory that exists and could not be read, and that now throws. Three test fixtures caught this by failing on a temp root with no `autobot-slm-frontend`. 3. The rule matched import paths by pattern instead of resolving them, and I had already got this wrong twice in opposite directions: - unanchored, `tokens.css` matched `design-tokens.css`, which IS imported, so the unloaded file read as loaded -- and telling those two apart is this rule's whole job; - anchored on a preceding `/`, it then missed a legitimate same-directory `@import "tokens.css"` and could still match a same-named file elsewhere. Specifiers are now resolved against the importing file's directory and compared as absolute paths, which has neither failure. The keyword is captured because it decides what a BARE specifier means: in CSS `@import "tokens.css"` is relative to the stylesheet, in JavaScript a bare specifier is a package. Treating them alike is what produced the miss. Five tests added, 22 passing, up from 17: a file outside every TARGETS exits 2, a same-directory `@import` counts as loaded, a same-named file in another directory does not, and a prefixed name is not a match. Verified: `--all` exits 0 over a clean tree; an uncovered `--files` exits 2; a covered file exits 0. A fourth thing, found by the fix breaking its own commit: the refusal above was too blunt. The pre-commit hook runs `canonical_check.mjs --files` over every changed `autobot-frontend/**/*.{ts,vue,mjs,js}`, which includes this harness, which no rule targets -- so exit 2 failed the very commit implementing it. The two modes differ in what "nothing in scope" means. `--all` chose the files itself, so nothing matched means it inspected nothing and success would be a false clean bill: it refuses. `--files` is a filter and the caller chose the files: it reports what it skipped and exits 0. That keeps the reviewer's point -- an audit cannot report clean over an empty inspection -- without breaking a per-file hook whose job is to skip what it does not cover. * fix(ci): a deleted CSS file made stylelint check nothing at all (#17571) The stylelint job builds its file list with `git diff --name-only`, which reports DELETED paths, and hands that list to `npx stylelint`. stylelint answers NoFilesFoundError for a path not in the checkout and exits 123 *before* linting any of the files that do exist -- so a PR removing one CSS file received zero lint coverage on the CSS it actually changed. `continue-on-error: true` then rendered that as an ordinary red badge, which is the reading the guard doctrine exists to forbid: "ran and found problems" and "never opened a file" produced the same signal. This PR hit it by deleting `autobot-frontend/src/assets/tokens.css`. `--diff-filter=d` on both project diffs. Not a new idea in this repo -- `import-hermeticity-sweep.yml:123` already carries the flag with the same justification, so stylelint-tokens was the one workflow out of step, not a defect class needing a sweep. The guard runs the workflow's own `run:` script in a throwaway repo rather than grepping it for the flag, per `prepush_diff_range_16637_test.py`: a flag can be present and the pipeline still wrong. Three behaviours are pinned separately, because a script emitting nothing would satisfy the deletion assertion while checking even less than the bug did -- the deleted path is absent, the added and modified paths are still listed, and `frontend_count` (which gates whether the lint step runs at all) matches the filtered list. Verified against a throwaway repo before pushing: listed `['src/added.vue', 'src/kept.css']`, `frontend_count=2`, deleted path absent. * fix: restore 32 files my own `git add -A` deleted (#17571) Not a change I made or intended. `517b1aee97` shows 37 files changed and 16,590 deletions; three of those files are this PR's subject and the other 34 are collateral -- `.dockerignore`, `cosign.pub`, `.vscode/settings.json`, `autobot-infrastructure/.../redis.conf`, every `data/system_knowledge/` tool, procedure and workflow, `data/file_manager_root/`, the `.superpowers/` reports. Mechanism: those paths are TRACKED and also matched by `.gitignore` (`.gitignore:345` `.dockerignore`, `:93` `*.pub`, `:434` `**/.vscode/`). They were absent from this worktree, and `git add -A` stages a deletion for a tracked file missing from the working tree -- .gitignore only ever governs whether an UNTRACKED file is added, never whether a tracked one is removed. So a bare `git add -A` in any worktree missing a tracked-but-ignored file commits its removal silently. The "Detect Mass Deletions (#4111)" pre-commit hook reported Passed for all 34. This is also the cause of most of this PR's red, which had been read as one guard objecting to the deliberate `assets/tokens.css` removal: - `.dockerignore` gone -> all 6 of test_dockerignore_test_file_coverage_14127 (FileNotFoundError, not an assertion) - data/system_knowledge/... -> the #17089 phase-criteria path guard - redis.conf, .superpowers/ -> the glob-declaration guards Restored from `origin/main` and verified byte-identical per path. TWO FILES ARE NOT RESTORED HERE, and this is a blocker I am reporting rather than working around. `data/file_manager_root/test.py` (`print("Hello World")`) and `data/file_manager_root/test.js` (`console.log("test")`) are one-line file-manager UPLOAD FIXTURES whose content is the point of them. Restoring them is refused by the `No print()/console.* in Production Code (#1082)` pre-commit hook, which scans staged files and does not know these are data: COMMIT BLOCKED: 2 print()/console.* violation(s) found The exemptions available are `# noqa: print` / `// noqa: console`, which would change the fixtures' contents so they no longer match `origin/main`, and a path allowlist, which that hook's own comments (#16008) argue against at length as "an exemption with no allowlist, no baseline and no comment". Neither is mine to choose unilaterally, so the two files stay deleted and stated rather than quietly patched. `pre-commit-no-print-console_test.py::TestScanCostAndRepoWideResult` is expected to stay red until they are back. Refs #17571 * fix: restore the five remaining files the same `git add -A` deleted (#17571) The tail of the previous commit's restore. These five needed `git checkout origin/main -- <path>` rather than `git add -A -- <path>`, because they were absent from the working tree entirely, so there was nothing for `add` to stage: autobot-infrastructure/autobot-database/templates/redis.conf data/system_knowledge/procedures/evidence_handling.yaml data/system_knowledge/workflows/image_forensics.yaml data/system_knowledge/workflows/incident_response.yaml data/system_knowledge/workflows/network_analysis.yaml Those absences are what the glob-declaration guards were reporting during the pre-push run -- `slm_frontend_atomic_publish_15610_test.py` names the redis.conf template and `generate_service_keys_export_dir_16348_test.py` names evidence_handling.yaml. I read that output as an informational listing of affected guards. It was the deletion being announced, one path per line, and I walked past it twice. 32 of 34 now restored and byte-identical to `origin/main`. The two upload fixtures carrying a `print(` and a `console.log(` remain deleted and are reported in the previous commit rather than worked around. Refs #17571 * fix(repo_tests): a fixture path became a real file, and main is green without noticing (#17571) `prepush_frontend_projects_17574_test.py` feeds sample paths to the pre-push hook's filter. They must NOT resolve: `python_filter_covers_its_guards_test` records any quoted path that resolves as a tree this guard depends on, and the remedy for that is an entry in a shrink-only record which nothing can later remove. The docstring already carried two constraints pulling against each other. A third arrived from the tree rather than from a guard. The paths were `Chat.vue`, `useThing.ts`, `RolesView.test.ts`, `Button.vue` -- ordinary names in ordinary directories. #17616 created `autobot-slm-frontend/src/views/RolesView.test.ts` and `.../composables/usePerformanceMonitoring.test.ts` for entirely unrelated reasons, and two fixtures became real. That breaks this guard AND pushes the uncovered-reads record from 45 to 47. MAIN IS CARRYING THIS BREAK RIGHT NOW AND ITS BADGE IS GREEN. #17616 changed only `autobot-slm-frontend/**`, which the python-paths filter does not cover, so `python-suite` was SKIPPED on `bf6ee5583` -- every shard, reported as `skipped`, run conclusion `success`. The bypass that let it through is the exact one `python_filter_covers_its_guards_test` exists to name: "a change confined to one takes the required-context shim's green while the guard never runs." The guard predicted the hole, the change went through the hole, and what it landed on was the guard. Nothing was wrong with either; the two never met, because the suite holding one of them did not run. Fixed here rather than in a new PR because this PR was already red on it and unblocking base is worth more than tidy attribution. - Every fixture path now carries the reserved `__prepush_fixture__` segment. Project prefix and extension are unchanged, because those are what the hook's filter routes on; only the basename moves. - `test_every_fixture_path_is_reserved_by_name` makes not-resolving a property of the NAME. `test_no_fixture_path_names_a_real_file` checks the tree as it stands, which is only ever a statement about the present -- it passed for months and then someone added a file. The reserved segment is independent of what anyone adds next. - The composable-sibling test builds its sibling under `tmp_path`, so its name tracks the input for matching only; nothing there touches the real tree. Also: two reach floors, MEASURED in this tree rather than copied from #17610 which ratchets the same two: blanket-skip-test-module-sweep 2540 -> 2741 test modules collectable-modules-inert-on-import 2527 -> 2728 matching modules Ratchet baselines are hub files and exempt from one-PR-one-owner; whichever of the two PRs merges second resolves a one-line conflict. Verified: 318 passed, 0 failed across prepush_frontend_projects_17574_test.py, python_filter_covers_its_guards_test.py and reach_declarations_test.py. Refs #17571 * fix(hooks): data/ is application data, so the print()/console guard stops scanning it (#17623) * fix: restore the last two deleted files — the guard's domain was wrong, not its rule (#17623, #17571) Completes the #17622 restore: 34 of 34 back. 33 are byte-identical to `origin/main`; `data/file_manager_root/test.js` differs by one trailing newline, which the `fix end of files` pre-commit hook adds and which I am not going to bypass a hook to avoid. Saying "byte-identical" of all 34 would have been the easier sentence and a false one. The two upload fixtures could not be re-committed because the `No print()/console.* in Production Code (#1082)` hook refused them. I read that as a policy question for a human -- `# noqa` markers that would change fixtures which must match base, or a path allowlist that the hook's own #16008 comments argue against. Both of those were the wrong framing. The rule is right; the DOMAIN is wrong. `data/` is application data, not production code -- `file_manager_root` is a registered data path via `PathsManager.get_data_path("file_manager_root")` (`paths_manager_test.py:49`). The whole tracked tree holds exactly two source-like files under `data/`: $ git ls-files 'data/**/*.py' 'data/**/*.ts' 'data/**/*.vue' 'data/**/*.js' data/file_manager_root/test.js data/file_manager_root/test.py Both one-line upload fixtures whose `print("Hello World")` and `console.log("test")` ARE the sample being uploaded. This is not the exemption #16008 removed. That one was invisible -- it lived in the gap between the scan's scope and nobody editing those lines. This excludes a DIRECTORY for what it is, beside `autobot-infrastructure/`, `code_analysis/`, `ansible/` and `tools/lint/`, which the same two greps already exclude with their reasons inline; and it hides nothing, because those two files are all there is to hide. Credit to f8 for the reframing. Two things worth recording for whoever meets this next: * The fixtures miss every filename exemption by one character. `test.py` is not `_test.py` and not `test_*.py`; `test.js` is not `*-test.js`. * Re-adding them needed `git add -f`. They are `.gitignore`-matched, and once the deletion landed they stopped being tracked, so a plain `add` refuses them. That is #17622's mechanism running backwards: the same ignore rule that let them be deleted without notice now stands in the way of putting them back. RE-MEASURED, and the arithmetic does not close: declared before 301 minus the two fixtures that left the domain -2 expected 299 MEASURED 300 The extra one is not from this branch: no file it touches violates, and the 32 files it restored are yaml/json/config this scan never reads. So base has been one violation short of its own pin, and nothing reported it -- `python-suite` is SKIPPED on any PR touching no python path and the run still says `success` (#16087). That is the third thing today to reach `main` through that gap. Pinned to the measured 300, not to the expected 299. Verified: 86 passed, 0 failed across pre-commit-no-print-console_test.py, python_filter_covers_its_guards_test.py, glob_declared_reads_15900_test.py and prepush_frontend_projects_17574_test.py -- the last two because changing a scanned domain can move another guard's population, which it did not. Closes #17623 Refs #17622 Refs #17571 * fix(canonical): three review findings, all the defect this harness exists to catch (#17571) Every one of them is a clean result standing in for an inspection that did not happen, inside the harness built to make that impossible. A fourth turned up while fixing them, and it is the worst of the four. 1. ds_unloaded_authority.check() SWALLOWED ITS OWN READ FAILURE. `readFile` throwing returned `[]`, which means "this stylesheet is fine" -- for a stylesheet nobody read. The two traversal paths a few lines up already throw for exactly this condition; the function that does the actual reading did not. It throws now. 2. THREE TESTS SKIPPED THEMSELVES IF THE EXPORT VANISHED. `if (!isImportedForTest) return;` meant that dropping the export would make all three pass without creating a fixture or resolving a single specifier -- a green suite over an uninspected rule. Asserted now, not guarded. 3. THE RESERVED-NAME CHECK HAD NO CONTRAST CASE. It only ever sees paths this file chose, so it would pass identically if the rule were `assert True`. Added the name that actually broke things -- the real `RolesView.test.ts` #17616 created -- so the assertion can fail. 4. AND THE ONE I FOUND WHILE VERIFYING: `runner.test.mjs` HAS NEVER RUN. It imports from vitest and asserts on the harness, but vitest's `include` was `src/**` only and the file lives under `scripts/`: $ npx vitest run scripts/canonical/__tests__/runner.test.mjs No test files found, exiting with code 1 198 lines of tests that read as coverage in a diff and were executed by no job, local or CI. I had run them earlier by another route and recorded "22 tests pass", which is true and was never a statement about CI. `include` now also matches `scripts/**`; one file matches it today, and the point is that the next one is collected rather than ignored. Findings 1-3 from review on #17608. Finding 4 is the same family one level out: 1 is a rule reporting clean without reading, 4 is a suite reporting nothing without running. Verified: 22 passed in runner.test.mjs -- now through the config rather than around it -- and 36 passed in prepush_frontend_projects_17574_test.py. Refs #17571 * fix(repo_tests): the contrast case put the defect back that its own rule removes (#17571) CI red on `python_filter_covers_its_guards_test.py`, both tests, and the cause was the fix I wrote for review thread 3 on this PR. That thread was right: `test_every_fixture_path_is_reserved_by_name` only ever saw paths this file chose, so it would pass identically if the rule were `assert True`. I added a contrast case, and reached for the most rhetorically satisfying string available -- the real path that broke things: assert "__prepush_fixture__" not in "autobot-slm-frontend/src/views/RolesView.test.ts" A quoted path that RESOLVES is exactly what `python_filter_covers_its_guards` records as a tree this guard reads. #17616 created that file, so the literal resolves, so the uncovered record went back up -- 45 to 46, one short of the 47 the original break caused. The contrast case for the sentinel rule reintroduced the defect the sentinel rule exists to remove, in the same commit that introduced the rule. The real name now stays in prose, unquoted, and the assertion uses a path that resolves to nothing. The property under test is whether a name carries the sentinel; it never needed the name to be real. That was the whole point of the sentinel and I did not apply it to my own new line. MERGE RESOLUTION. #17615 landed its own repair of the same file, renaming the two slm fixtures to `ExampleOnlyView.test.ts` / `useExampleOnly.test.ts`. Resolved toward the sentinel, which is a difference in kind rather than degree: `ExampleOnly` is a name nobody has created YET and a sentinel cannot ever be a real path. All six fixtures carry it, not just the two that happened to break. FLOORS: MEASURED, NOT RECONCILED, AND NOT CHANGED. blanket-skip-test-module-sweep population 2744, floor 2743, headroom 1 collectable-modules-inert-on-import population 2731, floor 2730, headroom 1 allowance 200 on both Taken from `len(REACH.discover())` in each declaration, not from a grep. #17615 merged 2743/2730 into base measured against a tree with its two new modules; this branch adds one more, so each population sits one above base's floor and 200 below the allowance. The floors already cover it, so they are left alone -- raising them would be churn that collides with every other branch measuring the same pair. My earlier 2741/2728 described a tree that no longer exists. Verified: 49 passed, 0 failed across python_filter_covers_its_guards_test.py and prepush_frontend_projects_17574_test.py; reach_declarations_test.py green on both declarations. Refs #17571 --------- Co-authored-by: t <t@example.invalid> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

mrveiss added 2 commits September 27, 2026 09:49

Merge branch 'main' into issue-17558-floor-report-roots

08025b3

mrveiss added 3 commits September 27, 2026 13:34

Merge remote-tracking branch 'origin/main' into issue-17558-floor-rep…

3303ace

…ort-roots

Merge remote-tracking branch 'origin/issue-17558-floor-report-roots' …

5353afa

…into issue-17558-floor-report-roots

mrveiss mentioned this pull request Sep 27, 2026

feat(canonical): the audit harness reported clean over a tree it never read (#17571, #14785) #17608

Merged

Merge branch 'main' into issue-17558-floor-report-roots

be3964b

coderabbitai Bot reviewed Sep 27, 2026

View reviewed changes

Comment thread pipeline-scripts/check_dependency_floors.py Outdated

Comment thread pipeline-scripts/dependency_floor_report.py

Comment thread tools/lint/check_ci_plane_unenforced_pins.py Outdated

Comment thread tools/lint/check_ci_plane_unenforced_pins.py Outdated

mrveiss and others added 3 commits September 27, 2026 16:35

Merge remote-tracking branch 'origin/issue-17558-floor-report-roots' …

8c6f9e7

…into issue-17558-floor-report-roots

Merge branch 'main' into issue-17558-floor-report-roots

2aa95b8

This was referenced Sep 27, 2026

ci: two green PRs merged six hours apart broke six checks in combination — the merge queue that prevents this is wired and disabled #17636

Closed

Enable the merge queue — no PR merges against a stale base #15937

Open

coderabbitai Bot reviewed Sep 27, 2026

View reviewed changes

Comment thread pipeline-scripts/check_dependency_floors.py Outdated

Comment thread pipeline-scripts/dependency_floor_report.py Outdated

mrveiss merged commit 6d61b87 into main Sep 27, 2026
68 checks passed

mrveiss deleted the issue-17558-floor-report-roots branch September 27, 2026 16:35

mrveiss mentioned this pull request Sep 27, 2026

deps(guards): two checks report clean over what they never compared — absent packages and one-plane-only pins #17558

Closed

8 tasks

Navigation Menu

Repository navigation

fix(deps): a floor report states its role and scope; detect a production bound the CI plane cannot keep (#17558) - #17610

fix(deps): a floor report states its role and scope; detect a production bound the CI plane cannot keep (#17558)#17610
mrveiss merged 13 commits into
mainfrom
issue-17558-floor-report-roots

mrveiss commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Sep 27, 2026

Uh oh!

mrveiss commented Sep 27, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrveiss commented Sep 27, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mrveiss commented Sep 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thinking Path

What Changed

Verification

Model Used

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Sep 27, 2026

✅ SSOT Configuration Compliance: Passing

Uh oh!

mrveiss commented Sep 27, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrveiss commented Sep 27, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mrveiss commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading