Repository navigation
bug(ci): no-local-schemas demands a move into modules that are all at zero headroom, and fails PRs for violations they did not introduce #16178
Description
Activity
- added a commit that references this issue
on Sep 10, 2026 Three witnesses now, not one — and they are three different hooks
This issue was filed about
no-local-schemas. In one session, the same structural
failure blocked three separate commits through three different hooks, each
failing a PR for a violation it did not introduce:hook what it flagged measured no-local-schemas_RS256RevokeRequestinapi/auth.pypresent on base at :240, untouched by the diffflake8 E501a 143-char line in python_file_size_ratchet_baseline.pypre-existing; the diff changed a different key function-length-checkanalyze_diffatapi/analytics_code_review.py117 body lines on base AND branch, threshold 65, and the diff never touches that function The third is the cleanest evidence: byte-identical function on both refs, zero
hunks mentioning it, and it blocked a two-line security fix — routing a git call
through the canonical scrubbed runner for #16179.Why this is one defect and not three
Each hook reads the changed file whole rather than the changed lines. So the
cost of touching a legacy file is that file's entire backlog, and the cost scales
with how long the file has existed rather than with what the change does.#13950already fixed exactly this for the hardcoded-values hook, and the workflow
comment records what it cost before the fix:"this hook reads each changed file whole, so editing one line of a legacy module
made the PR answerable for that file's entire backlog — four PRs in one session
failed on violations blame attributes to other commits."Four PRs in one session then; three commits in one session now, across three
hooks that never got the same treatment.The perverse consequence, concretely
A security fix is more likely to be blocked than a feature change, because
security fixes land in old, large, much-touched files — exactly the files carrying
the most inherited debt. Tonight that meant:api/auth.py— blocked, worked around by finding a schemas module with headroomanalytics_code_review.py— still blocked, so one of seven unscrubbed git call sites remains unscrubbed
The one still standing is not blocked on difficulty or on review. It is blocked on
a 117-line function somebody wrote before the fix existed.Scope note
The ceiling half of this issue dissolved and is worth recording as a
non-finding:run_git()fromautobot_shared/git_probe.pymade two of the three
files smaller, so no grandfathered ceiling needed raising after all. The
whole-file-hook half is what remains, and it is the half with three witnesses.Owner decision on AC3: companion modules, no big-bang split
Put to the owner and answered (2026-09-11). The frozen domain modules stay frozen and are not re-split. New models go to per-subdomain companion modules that have headroom. Several already exist on base:
schemas_agent_requests.py(108 lines),schemas_chat_rows.py(109),schemas_canvas.py(130),schemas_analytics_collector.py(66) and others. A frozen module shrinks only when a model moves out of it while that code is being worked on.Rejected: a planned re-split with
tools/schema-split, as #5799 did. It would rewrite imports acrossapi/in one large change, with a lot of conflicts against open PRs, to fix a problem the companion pattern already fixes one model at a time.What this means for AC2: the hook's message will compute each
schemas_*.pymodule's headroom from the file-size guard's own ceiling data, not a hard-coded list. It will name the companions with headroom, and mark frozen modules as "frozen, use or create a companion". That replaces today's advice to move the model into one of seven frozen files.AC3 will be ticked once the convention is in merged code, in the hook's docstring and message. This comment is a decision, not the durable record.
Tracked under umbrella #16238, which covers every hook that reads a whole changed file.
Implementation plan
- The hook scopes itself to added lines.
check_no_local_schemas.pygets--changed-lines-only. With no--baseit reads the staged diff, which is the local commit stage, where tech-debt(security): two PasswordChangeRateLimiter classes with different policies and one name #15757 was blocked. With--base REVit readsREV..HEAD, which is CI. A model counts as new only if itsclassline was added. If git fails, the hook fails closed. Pre-existing hits are still listed, as not counted, so the backlog stays visible. - Why the hook and not the CI wrapper.
check-pre-commit-hook-pr.shonly diffs commit ranges (base..HEAD), and a staged change has no SHA, so it can't serve the local stage. ci(code-quality): the hardcoded-value guard scans changed FILES, not changed LINES — three unrelated PRs went red in one session #13950's precedent deliberately left the local stage whole-file. This hook has no baseline file andnever runs in CI(corrected: it runs in CI only as a warning, viaenforce-precommit.yml's all-hooks pre-commit run), so the local stage is exactly where it has to change. - Shared helper.
added_lines()goes intools/lint/_scan_helpers.py, the existing module that already runs git withscrubbed_git_env().No Python hunk parser exists anywhere in the repo, which I checked first.Corrected: two exist,_HUNK_HEADER_REinautobot-backend/api/analytics_code_review.pyand inautobot-backend/code_intelligence/code_review_engine.py. My search coveredtools/,scripts/,pipeline-scripts/,autobot_shared/andrepo_tests/, not the backend, and I stated it as "anywhere". They parse LLM-supplied diffs inside the backend, whichtools/lintdoes not import, so not reusing them still holds. - Wrapper defect fixed.
--changed-lines-only --python Xis currently accepted and silently ignored: validator mode never scopes. It will pass--changed-lines-only --base <resolved base>to the validator instead. A validator that can't scope then rejects the unknown flag loudly, instead of silently running unscoped. - AC2 message. The per-hit guide naming seven frozen files is replaced by a table of every
schemas_*.pymodule's headroom. It's computed from the size guard's ownMAX_LINES,KNOWN_LARGEandcount_lines, never a copied number. Frozen modules are marked "use or create a companion", per the AC3 ruling. - AC3. The companion-module convention goes into the hook's docstring, the durable record.
- AC4 tests run against a real throwaway git repo. An unrelated staged edit to a file carrying a pre-existing model passes. A newly added model fails. The same pair runs in
--basemode. Also tested: the wrapper passing the flags through. - Pending the coordinator's OK:
.pre-commit-config.yaml(addargs: [--changed-lines-only]) andcode-quality.yml(a scoped CI step; the hook currentlynever runs in CI(corrected: it runs in CI only as a warning, viaenforce-precommit.yml's all-hooks pre-commit run)).
A direct
python3 tools/lint/check_no_local_schemas.pystays whole-file.
Edited after review: the hunk-parser claim above was false as stated and is struck through rather than removed, because it was the narrower search's answer reported as a broad one. The review also found that the helper couldn't see renames, and that is fixed in #16241.
Second edit: "never runs in CI" was false. I grepped workflows for the hook's name, and
enforce-precommit.ymlruns every hook,pre-commit run --from-ref HEAD^1 --to-ref HEAD^2, without naming any. Finding it also exposed a real defect in #16241. Under--from-ref, pre-commit stages nothing, so a staged-mode diff is empty and every model would read as pre-existing. The hook now usesPRE_COMMIT_FROM_REFas its base, and it judges an unstaged file whole-file rather than as "nothing added".- The hook scopes itself to added lines.
- added 4 commits that reference this issue
on Sep 11, 2026 - added a commit that references this issue
on Sep 12, 2026 Closing: all 4 criteria met on
Dev_new_gui(4a7e76e28), delivered by #16241.AC Evidence 1. Scoped to changed lines at PR stage tools/lint/check_no_local_schemas.py:20-22--changed-lines-only [--base R]2. Message names headroom, or frozen-needs-split :208-225_headroom_rows/guidance function3. Split-vs-companion-modules decided and recorded Owner decision comment on #16178 (companion modules, no re-split), recorded in the hook's docstring 4. Verified: pre-existing violation doesn't fail, new one scoped does tools/lint/check_no_local_schemas_test.py:298test_an_unrelated_staged_edit_is_not_blamed_for_a_preexisting_model;:312test_a_newly_added_model_still_fails_when_scoped- added a commit that references this issue
on Sep 18, 2026
no-local-schemasrequires a move; every destination it names is frozenThe hook blocks a local
BaseModelin a non-schema API file and instructs:Measured 2026-09-10 on
origin/Dev_new_gui:schemas_agent.pyschemas_analytics.pyschemas_chat.pyschemas_code.pyschemas_knowledge.pyschemas_system.pyschemas_workflows.pyEvery one is at its grandfathered ceiling, and the file-size guard says: "A
grandfathered file may not grow (#14236) — the exemption freezes the size it was
granted for, it does not license more."
So the two rules are jointly unsatisfiable for the destinations the hook names.
How it surfaces, which is the part that costs time
The hook reads each changed file whole. Editing one unrelated line of
api/auth.pyfor #15757 made that PR answerable for a pre-existing localBaseModelatauth.py:239— present on base, blame belonging to another commit.This is the same shape #13950 fixed for the hardcoded-values hook, whose workflow
comment records the cost:
no-local-schemasnever got that treatment.The workaround found, which is not a fix
schemas_agent_requests.py(93 lines, ceiling 600, +507 headroom) accepted themodel. Several such split-out companions exist —
schemas_agent_requests,schemas_chat_rows,schemas_analytics_collector— and they are where new modelscan actually go.
That works and is arguably better design, but it is discovered by measuring every
schemas module's headroom, not by reading the hook's message. The hook names
seven files, all of which refuse the move.
Acceptance criteria
no-local-schemasscopes to changed lines at PR stage, as ci(code-quality): the hardcoded-value guard scans changed FILES, not changed LINES — three unrelated PRs went red in one session #13950 did for hardcoded-values — a pre-existing violation must not fail a PR that did not introduce itschemas_*.pymodules should be split is decided and recorded — several are 3,000–5,000 lines, which is what put them behind a ceilingNot in scope
Raising any ceiling.
auth.pygot smaller in #15757 and the ceiling was loweredto match; that is the direction the ratchet is for.