Repository navigation
ci: code-metrics gate on touched files with baseline-free checks; fold in import-linter, PR hygiene and ruff principle rules - #1812
Open
zoroyihan7 wants to merge 38 commits into
Open
zoroyihan7 wants to merge 38 commits into
zoroyihan7 wants to merge 38 commits into
Conversation
Code metrics gate: PASSEDJudged: the 150 files this change touches. Files it does not touch never fail it; anything found there is listed under Outside this change for information.
Module length warning: over 800 lines, within the 1200-line failure limit (not failing): 8
Already over at the base (backlog, not this change's): 4
8 findings in files this change does not touchOutside this change (informational): 8
Thresholds and sources: |
zoroyihan7
force-pushed
the
ci/code-metrics-gate
branch
from
October 10, 2026 12:23
5048fb4 to
bf0caf7
Compare
Adds the `code-metrics` CI job. It measures cyclomatic and cognitive complexity, statements/branches/returns/arguments/locals/nesting per function, public methods per class, module length, maintainability index, duplication and dead code at each tool's industry-default threshold, over src/hyperloom, src/kernelforge and scripts. Units already over a threshold are recorded at their current value in scripts/code_metrics_baseline.json and may only go down: a new violation fails, a worse baselined unit fails, an improved or removed one fails until `python scripts/code_metrics.py --update-baseline` tightens the baseline, and the baseline and config may not grow or loosen relative to the base branch. The report goes to the job summary and to one sticky PR comment; fork PRs are commented on by a separate workflow_run workflow that treats the uploaded report as data. Co-Authored-By: Claude <noreply@anthropic.com>
A move was matched on the last name segment and greedily, so a worse unit could take over the allowance of a same-named one (B.run moving next to an improved A.run, or two main functions). Match on the qualified name (file name for module metrics) and only when exactly one removed and one added key share it. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Without a base ref, a push straight to main (an admin bypassing PRs) could raise a baseline value or loosen a threshold unnoticed. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
zoroyihan7
force-pushed
the
ci/code-metrics-gate
branch
from
October 10, 2026 12:41
bf0caf7 to
49ec9c1
Compare
Resolve the docs conflicts with #1811's complexity ceiling: the review ceiling stays, and the code-metrics gate is described as stricter inside its scope. Co-Authored-By: Claude <noreply@anthropic.com>
- roots/exclude must be plain repository paths, git reads them with --literal-pathspecs, and loosening is judged by the files each config measures in the head tree, so a ':(exclude)' root can no longer drop a file from the gate while reading as a widened scope. - CI runs the base branch's copy of the gate scripts (and comment poster) over the change, so editing scripts/code_metrics*.py does not change the verdict on the same PR; the report lists edits to the gate and its tool pins under "Gate implementation changed". - vulture and jscpd read copies with '# noqa' and 'jscpd:ignore' markers defused, so the documented "suppression comments do not hide a unit" holds for dead code and duplication too. - The scope is all of src (hyperloom_kb was outside it) and scripts; the baseline is re-seeded on the merged tree: +45 entries, 39 for hyperloom_kb and 6 that '# noqa' had hidden from vulture; no existing entry changed. Co-Authored-By: Claude <noreply@anthropic.com>
Add .importlinter with seven contracts: hyperloom top-level layers, kernelforge depends on hyperloom.common only, hyperloom.common is a leaf, agent packages and kernel backends are independent, only the orchestrator and hyperloom.cli import kernelforge directly, and no cycles between sibling packages. Existing violations are recorded as exact ignore_imports lines (56 upward imports of hyperloom.inference_optimizer.cli, 155 cycle breakers) with unmatched_ignore_imports_alerting = error, so the baselines can only shrink. Run it as a hard gate in lint.yml (import-linter==2.15 pinned) and as a local pre-commit hook; document the rules in the style guide. Co-Authored-By: Claude <noreply@anthropic.com>
Make G, TD001/TD003-TD007, FIX001/FIX003/FIX004, PGH, RSE, ERA, PIE, RET, DTZ, LOG, A and C4 hard gates via [tool.ruff.lint] extend-select, after clearing the current findings: - safe autofixes (PIE807/PIE790/PIE808, C420, RET501/RET502/RET505, RSE102), then reviewed unsafe fixes for C401/C405/C416/C408, PIE810 and RET504 (all behaviour-preserving rewrites); - ERA001: every hit was a prose comment that parses as Python; the comments are reworded, no code is deleted; - PGH003: name the ignored code (import-not-found), as elsewhere; - LOG014: pass the exception in scope instead of exc_info=True outside a handler; - documented per-file ignores where the old shape is behavioural: naive persisted timestamps (DTZ), stdlib-mirroring keyword names (A002), root-logger calls (LOG015), Sphinx `copyright` (A001); tests may use dict(...) and builtin-named fake kwargs. `aiter` (AMD's library) is allowlisted for A. Also align the pre-commit ruff hook with CI's pinned 0.16.2, drop the CODEOWNERS entry for /setup.py (removed in #808; root *.py is already covered by /*.py), and update the style guide's lint section. I (isort) and UP stay off pending a team decision on landing the mass rewrite. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
- tests-coverage.yml: on pull requests, the Python 3.10 coverage job writes coverage.xml and runs diff-cover 9.7.1 against the merge commit's base parent with --fail-under=80; the markdown report goes to the job summary. - pr-hygiene.yml (new, always runs, read-only token): diff budget over production Python lines (src/, scripts/, tests excluded; warn > 400, fail > 1000 unless a writer added the size-exception label), the agent-doc CLI reference check moved from docs.yml, and an advisory PR template completeness check that reads the body only as data. - scripts/pr_hygiene.py + tests pinning each refusal message. Co-Authored-By: Claude <noreply@anthropic.com>
A pure rename has zero numstat lines, so moving a 1001-line test file to a production path passed the diff budget at 0 lines. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
… text diffs check_cli_references.py prints PR-controlled file paths; a crafted path could start a workflow command. A PR's .gitattributes marking *.py as -diff hid its lines from both the budget and diff-cover; .git/info/ attributes takes precedence over it. Co-Authored-By: Claude <noreply@anthropic.com>
…result - Dead code: vulture reads the real files again, so a `# noqa: F401` side-effect import or re-export (Ruff's sanctioned form, with RUF100 flagging unused ones) is not forced into an importlib rewrite. jscpd still reads copies with `jscpd:ignore` defused. Baseline tightened with --update-baseline: 3412 -> 3406 entries (6 dead-code entries removed, none added or raised). - Fork poster: the comment opens with the conclusion, head SHA and run link taken from github.event.workflow_run, above the artifact text, so a fork-uploaded "PASSED" cannot misstate the result. Cancelled/skipped runs are not posted, a run with no artifact is a notice, and one poster runs per fork branch. - A refused comment (403, 5xx, rate limit) or artifact upload is a warning; the job result comes from the gate step alone and a gate crash stays red. Co-Authored-By: Claude <noreply@anthropic.com>
…seline Rework the code-metrics gate: - Dimensions: cyclomatic complexity > 20 (the style guide's complexity ceiling, now measured), cognitive complexity > 30, function length > 80 physical lines, nested blocks > 5, module length > 1200 lines (> 800 is a report-only warning), duplication and dead code unchanged. Drop the maintainability index and the pylint count dimensions. - Scope: only files the change touches are judged; findings elsewhere are informational. A unit the base already had at that value or worse is backlog, as the complexity ceiling defines it. Baseline growth stays a whole-file check. - The baseline-raise PR label, read from the API at run time, waives new, worse and growth findings, which stay listed in the log and the report. - New checks without a baseline: added comment blocks over 8 lines and PR/issue/incident history in added comments, CJK characters in tracked files and in the PR's title, body and commits, production imports of test code, and added repeated literals. - Baseline is one sorted line per entry in scripts/code_metrics_baseline.txt. - Fix the three files that carried CJK characters, and move the InferenceX anchor-contract helpers the refresh script imported from a test module into a production module both use. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Swapping an entry to a same-named new unit while the recorded one is still over the limit was matched as a move, so the baseline stayed the same size while the tree gained a second violation. Co-Authored-By: Claude <noreply@anthropic.com>
…seline line The baseline path comes from the PR's pyproject.toml and the job holds a token; a baseline set to /proc/self/environ (or a symlink to it) made the parse error quote the environment into the PR comment. The baseline must be a plain repository path and a regular file, pyproject.toml must not be a symlink, measured files skip symlinks, and a malformed baseline line is reported by number only. Co-Authored-By: Claude <noreply@anthropic.com>
…Python A PR could replace code-metrics/ with a symlink into the runner temp directory and have upload-artifact follow it, uploading the persisted git credential. The report and the PR number now live under RUNNER_TEMP. A symlinked .py in scope was skipped, so its code escaped measurement; it is now refused (exit 2) without being read. Co-Authored-By: Claude <noreply@anthropic.com>
zoroyihan7
requested review from
Ahmedhasssan-aig,
chao-xu-spec,
devalshahamd,
iraj465,
tsrikris and
yueliu14
as code owners
October 10, 2026 18:29
…ract The helpers moved out of the test module into a production module, where diff coverage now grades them; pin build_record's record and each refusal, and fetch_pinned_file's fallbacks. Co-Authored-By: Claude <noreply@anthropic.com>
The module-length rule warns above module-lines-warning and fails above module-lines, but the report printed only "fails > 1200" in the dimension table and in the Limit column of the warning section, so the warnings read as failures. The dimension row now reads "warns > 800, fails > 1200", the warning section title names both lines, and its Limit column shows the warning threshold; both numbers come from the config. Co-Authored-By: Claude <noreply@anthropic.com>
… and concatenation The module-length warning applies to lengths over module-lines-warning and at most module-lines, so a warning value at or above the failing length can never warn; the dimension row then states only the failing level. Wrap the multi-line how-to-fix bullets in parentheses so the string concatenation is explicit, and import each module one way per test file (code_metrics_collect, _inferencex_anchor_contract), as the code-quality scan asked. Co-Authored-By: Claude <noreply@anthropic.com>
zoroyihan7
added a commit
that referenced
this pull request
Oct 11, 2026
A1-A14 were replayed on 30 recent merged PRs and every fire was judged by an independent adversarial judge. Severity now follows the measured precision: a rule or delimited sub-case is blocking at precision >= 0.8 on at least two real findings with an AGENTS.md sentence behind it. - A1: demoted to advisory as a whole (5/10); the added non-test import sub-case stays blocking (5/5). Tests and unchanged lines are carved out. - A2 stays blocking (2/2); A8(a) keeps blocking (no fire). - A12 pure forwarder (2/2) and A13 redundant re-check (2/2) promoted. - A4, A5, A9, A10, A11 get Not-a-finding clauses for each replayed false positive; every Severity line records its count. - Rules whose shape the code-metrics job (#1812) mechanizes say so. Co-Authored-By: Claude <noreply@anthropic.com>
zoroyihan7
added a commit
that referenced
this pull request
Oct 11, 2026
…lause to V1 The code-metrics job in #1812 (head 0962cff) checks none of the A shapes, so the Mechanized notes cited a gate that does not exist. The A11 clause no longer exempts an in-place resolve_* by convention; it covers only a write that moved unchanged from the parent (V1). Co-Authored-By: Claude <noreply@anthropic.com>
New scripts/code_metrics_design.py, run by the gate next to the comment and literal checks. Each judges only what the change adds (a moved, re-indented or renamed line is not added; a function or class is new only when the base file had no unit of that qualified name), skips test code, and has its own report section, refusal message and how-to-fix line: - A1 private-name reach: an added import or attribute access of a private name owned by another package unit; the units are the root packages, layers and independent modules of .importlinter. - A8 flag argument: a new function whose first statement branches on a bool or two-value Literal parameter into two bodies (CLI entry points exempt). - A9 tuple return: a new function returning a tuple of more than tuple-return-max-elements (2) positions, by annotation or by display. - A10 raw vocabulary: an added comparison with the string value of a member of exactly one Enum under vocabulary-roots, in a module that names that enum. - A11 hidden global write: an added global statement. - A12 forwarder and single implementation: a new function that only passes its parameters to another call; a new ABC or Protocol with one implementation in its own package unit. Config: import-contracts, vocabulary-roots and tuple-return-max-elements in [tool.hyperloom.code_metrics]; the last may not be raised and a vocabulary root may not be dropped. The gate's own tree now passes them: the anchor-contract helper module and its repository default become public, Finding.key returns a Key NamedTuple, and the violates() forwarder is gone. git output is decoded with replacement, so a binary file in the diff no longer stops the checks. Co-Authored-By: Claude <noreply@anthropic.com>
…d a missing contracts file Co-Authored-By: Claude <noreply@anthropic.com>
…in the design checks Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
A8 reads a flag comparison with the constant on the left. A9 sees a triple inside Optional/Union and leaves dunders, whose shape the protocol fixes. A10 reads case patterns. A11 also catches a store into globals(). A12 matches builtins only by bare name (a method named like one is still a forwarder), and passes a method of a constant, a functools.wraps wrapper and a method overriding its base class. Co-Authored-By: Claude <noreply@anthropic.com>
…n the design checks A9 no longer counts a returned display when the annotation admits any length (tuple[int, ...]). A1 no longer resolves an attribute through an import its enclosing function shadows with a parameter or an assignment. Co-Authored-By: Claude <noreply@anthropic.com>
… their own shapes A1 reads the package units from the merge base's contracts file, and moving import-contracts is reported as loosening, so a change cannot redraw the units it is judged by. A10 judges a comparison only when the other side reads as the member (its .value, or a name whose words end like the enum's): the code converting raw input to the member has to compare the string. One enum defined in two modules is one vocabulary. A12 leaves decorated functions (a cache, a registration) alone. Co-Authored-By: Claude <noreply@anthropic.com>
This branch has not been deployed
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.
Description: what and why
One PR for the CI quality gates. It adds the
code-metricsgate and folds in three sibling PRs, merged in withgit merge(no rebase); they will be closed as superseded.Folded in
.importlinter(7 architecture contracts; two carry exact-line baselines that only shrink), theimport-linterjob inlint.yml, the pre-commit hook, style guide "Dependency direction".tests-coverage.yml;pr-hygiene.yml+scripts/pr_hygiene.py(diff budget, template completeness) with tests;check_cli_referencesmoved fromdocs.ymltopr-hygiene.yml.extend-selectfamilies (G,TD,FIX,PGH,RSE,ERA,PIE,RET,DTZ,LOG,A,C4) held at zero with the fixes in 122 files, pre-commit ruffv0.16.2, the/setup.pyCODEOWNERS line removed, the style guide lint section.code-metrics: dimensions and why each number (
pyproject.toml[tool.hyperloom.code_metrics], every number commented there)defthrough the last line, decorators excluded, comments/docstrings included, a nested function is measured on its own, not folded into its parent; past one screenDropped: maintainability index (56 of 78 baselined files sat at MI 0, so worsening there was invisible; its comment term rewards adding comments, which the new comment check works against; no surveyed major Python repo gates on it), and pylint's statements, branches, returns, arguments, positional arguments, locals and public methods (function length replaces statements). radon is no longer pinned or installed.
Baseline:
scripts/code_metrics_baseline.txt, 1134 entries in 1138 lines (was 3406 entries in 8025 lines of nested JSON), one sorted line per entry,<metric> <path>::<Class.method> <value>(module metrics: the path alone), with a header stating the rule and the update command, so a baseline change is one diff line per unit. Seeded on this branch withpython scripts/code_metrics.py --seed-baseline, then tightened with--update-baseline.Scope: touched files only. A finding is judged only when its file is changed by the PR (diff against the merge base; for a push to main, against
github.event.before). Untouched files, baselined or drifted on main, never fail an unrelated PR and are listed under "Outside this change (informational)". In a touched file: a new or worsened unit fails (for function length: a function that newly exceeds 80 lines or grows while over it; editing a recorded long function without lengthening it is not a failure); a baselined unit that was fixed must leave the baseline in the same PR (out of date fails with the update command). New and worsened units are also compared with the same file at the merge base, so a unit the base already had at that value or worse is backlog, exactly D12's comparison. The "baseline may only shrink relative to the base" check stays whole-file.Override: the
baseline-raiselabel. Read from the GitHub API when the gate runs (--pr-number), never from the event payload. It turns baseline growth and new/worse findings into waived findings, still printed in full in the job log (untruncated) and in the report. It waives nothing else (out-of-date entries, config loosening and the checks below still fail). Use it only with the reason in the PR description. Adding or removing a label, or editing the title or body, re-runs the job (pull_requesttypes includelabeled,unlabeled,edited).Checks without a baseline (same job, same sticky report, each a section; each fails the job):
.pyfiles, docstrings excluded): an added run of more than 8 full-line#comments; an added comment with a PR/issue reference (#123+,PR 1234,PR-1234,GH-1234,issue #12, a/pull/or/issues/link) or incident narration (a dated incident or outage, "after the outage", "postmortem"). A comment starting withTODOand the line after it may link an issue (RuffTD003). The diff is read with-M -w --text --no-textconv, so a renamed file, a moved or re-indented comment is not "added" and.gitattributescannot hide hunks. Measured base rate on the last 80 commits: 2 commits had >8-line added blocks, 0 had PR references.scripts/ray_exec_p0_smoke.py,src/hyperloom/inference_optimizer/tests/test_inferencex_client_unit.py,src/hyperloom/orchestrator/enablement/tests/test_artifacts.py(multi-byte test data now uses the euro sign and Arabic-Indic digits). Absolute, no baseline.src/orscripts/imports atestspackage or atest_*module (an AST pass, so it coversscripts/, which import-linter cannot see;importlib.import_module/__import__with a literal name andconftestmodules count).kernelforge.mcp_server.tools.testis a tool module and does not fire. Backlog fixed here:scripts/refresh_inferencex_anchor_contract.pyimportedhyperloom.inference_optimizer.tests.test_inferencex_anchor_contract; the shared contract helpers moved tohyperloom.orchestrator.actions.executors.inferencex_anchor_contract(public, withINFERENCEX_REPO_DEFAULTmade public inpreflight, so the script and the module pass A1 below), used by the script and the test. Absolute.x["k"], the key of.get/.pop/.setdefaultand of anintest, keyword names, docstrings, f-string text, annotations,__all__and test files do not count. Diff-only on purpose: a whole-tree version hits 6089 literals in 479 of 832 modules.Design checks (diff-only,
scripts/code_metrics_design.py; each its own report section, refusal message naming the rule and the fix, and how-to-fix line; tests exempt from all). A line is added when the diff adds it and the change did not remove the same text elsewhere (the comment and literal checks'-M -wmachinery: moves, re-indents and renames are not additions); a function or class is new when itsdef/classline is added and the base file (or its rename source) had no unit of that qualified name. Replayed with the full gate over the 30 PRs merged just before (fix(orchestrator): bound the prompt sections that grow with the session #1735-fix(orchestrator): keep a PRELUDE skip_to_kernel hint from ending FRAMEWORK_AGENT #1808, merge commit vs first parent), the hit rate per check is:.importlinter, longest prefix wins,scripts/its own unit, units read from the merge base's contracts (movingimport-contractsis loosening); dunders and same-unit use exempthyperloom <command>entry point; remove dead CLI shells and fix CLI bugs #1736:scripts/check_cli_references.pyreadshyperloom.cli._COMMANDSand_SESSION_COMMANDS; true positive)bool/two-valueLiteralparameter (either side of the comparison) into a branch ending inreturn/raise;main, click/typer commands andargparse.Namespacehandlers exempttuple-return-max-elements(2) positions, by annotation (tuple,typing.Tuple, also insideOptional/Union/|) or display;tuple[X, ...](and the displays such a function returns) and dunders exempt==/!=/in-display comparison orcasepattern with the string value of a member of exactly oneEnumundervocabulary-roots(src), in a module that names that enum, against a subject that reads as the member (.value, or a name whose words end like the enum's:statusforRunStatus); raw input on its way to the enum (raw == "running") passesglobalstatement or store intoglobals();nonlocalpassesreturnof a call passing exactly its own parameters through; bare-name builtins, classes (factories),.getlookups, methods of a constant, decorated functions (other thanstaticmethod/classmethod) and methods overriding a base-class method exemptProtocolwith exactly one implementation (subclass, or for a protocol a class with all its methods) insrc, in the same package unit; an interface a lower unit declares for a higher one is exemptAcross the 30 PRs the replay saw 89 new functions and 4 new classes (cross-checked from git blobs). Narrowed with evidence: A10 first matched any enum value in
src; its one PR hit (feat(runtime-findings): scan server logs, surface findings to every role, allow verified correctness-fix KEEP #1793entry["status"] == "unknown"vsSpecialistFailureType.UNKNOWN) was another vocabulary reusing the word, and on the whole tree 165 of 173 hits were in modules that never name the enum, so it now needs a value unique to one enum and a module that names it (whole tree 173 -> 3); review then showed two of those three were the conversion of raw input or a version sentinel, so the compared side must also read as the member (whole tree 3 -> 1:recipe_journal.pycomparinggetattr(config.mode, "value")with"remote"). A12 forwarders skip builtins, factories and.get(whole tree 52 -> 43:SECTION_SHAPES.get(section),json.dumps(value)were accessors, not layers). Config in[tool.hyperloom.code_metrics]:import-contracts,vocabulary-roots,tuple-return-max-elements(raising it, or dropping a vocabulary root, is refused as loosening; a missing contracts file is "could not run"). On this branch the checks found four true positives in the gate's own new code, fixed here: the private anchor-contract module and constant above,Finding.keyreturningtuple[str, str, str](now aKeyNamedTuple), and aviolates()that only forwarded toworse(). Adversarial pass before push: the Optional/Union triple, the reversedLiteralcomparison,casepatterns,globals()[...]stores and a delegate method named like a builtin (self._client.next(job)) got past their checks and are now caught; afunctools.wrapswrapper, an adapter method overriding its ABC,", ".join(parts),__reduce__, a display returned undertuple[int, ...], an attribute of a parameter that shadows an import and an@lru_cachefunction were refused and now pass; A1 no longer trusts a contracts file the PR edits. Each has a test that is red without the fix. Rerun of the design checks alone over the last 30 first-parent commits of main with the final rules: A1 2 of 30 (refactor(cli): onehyperloom <command>entry point; remove dead CLI shells and fix CLI bugs #1736 above, andexplore.pyin f261cde importinggrid_server_args._MULTI_VALUE_FLAGSacross units, also a true positive), every other check 0 of 30. Also fixed:gitoutput is decoded with replacement, so a binary file in a PR's diff (read with--text) no longer stops every diff-only check with a decode error.Kept from before: the sticky PR comment, the fork
workflow_runposter with the conclusion header taken from GitHub, the base branch's copy of the gate scripts judging the PR (code_metrics_checks.pyandcode_metrics_design.pynow among them), scope-loss and pathspec refusals, best-effort posting.code-metricsstill runs on every PR (nopaths-ignore). The job holds a token, so nothing PR-controlled is read outside the checkout: the baseline must be a plain repository path to a regular file (a malformed line is reported by number, never quoted), a symlinked.pyin scope is refused rather than read or skipped, and the report and artifact are written underRUNNER_TEMP, not in the checkout.Admin actions
baseline-raise(it does not exist yet).size-exceptionwas created with this PR, which carries it: the diff budget counts 2,699 production lines, almost all of them docs(review-pr): add the A rule family for code shape, and the AGENTS.md principles it rests on #1815's mechanical Ruff fixes across 122 files, bundled on request.code-metricsandpr-hygieneas required status checks onmain. Ruff and import-linter live inlint.yml, which haspaths-ignore, so those two always-run jobs are the ones to require.require_code_owner_reviewandrequire_last_push_approvalin themainruleset: apull_requestrun uses the PR's own workflow file, so only CODEOWNERS review of/.github/workflows/stops a PR from replacing the gate steps.Known limits
baseline-raisemerge that skipped the baseline update), the next PR touching either copy sees it as new and needs the label or the baseline entry.#256-style counts in prose still read as a reference.[("A", 100), ("B", 100), ("C", 100)]count as three uses of100; name the value or carry the label.pull_requestgate, a PR can edit its own.importlintercontracts or[tool.coverage.run].omit; CODEOWNERS review of those files (admin action above) is what closes that, not the jobs.obj._xon an instance), A10 knows only string-valued enum members, and A12 matches a protocol's implementations by method names and an ABC's by base-class name.srcandscriptswithout tests (Ruff 0.16.2).Linked issue(s): supersedes ci(lint): enforce architecture contracts with import-linter #1813, ci: PR behaviour gates -- diff coverage 80%, diff budget, PR template check #1814, docs(review-pr): add the A rule family for code shape, and the AGENTS.md principles it rests on #1815
Tests: added/updated? commands run?
scripts/tests/test_code_metrics.py, newscripts/tests/test_code_metrics_checks.pyand newscripts/tests/test_code_metrics_design.py(285 tests; the design file asserts which check and line each case refuses, and that every exemption holds): each asserts which refusal fires (check name and message), including touched-file scope, out-of-date entries in touched files, the ceiling's head-versus-base comparison, the label read from the API (a forged event payload changes nothing), waived rows listed in full in the log, PR title/body/commit CJK, function-length counting, module-length warnings, and the workflow wiring. 52 guards mutated one at a time; each mutation turned its tests red for the stated reason, then restored. The design checks: 27 more guards mutated (each exemption, the added-line and new-unit rules, the wiring, the config refusal, the binary-diff decode), each turning its own named test red, restored from the commit.pytest -n 4 scripts/tests: 544 passed (before the design checks). With them: 659 passed, 6 skipped.ruff check .andruff format --check .: clean.PYTHONPATH=src lint-imports --no-cache: 7 kept, 0 broken.actionlint1.7.12 (with shellcheck) andyamllint1.38.0 on the changed workflows: clean.python scripts/code_metrics.py --base-ref origin/mainpasses in 27 s locally (the previous gate took 37 s of a 53 s CI job). End to end on scratch commits in a throwaway worktree: a CC 21 function in a touched file fails (CC 20 passes, the limit is "> 20"); a CC 25 function on main in a file the PR does not touch is informational and the PR passes; a 9-line added comment block fails; a CJK character in a.pyfails; a production import of a tests module fails; a literal written a third time fails; with a simulatedbaseline-raiselabel a new violation and a baseline growth pass as waived findings, printed in the log. Replays: renamingcategory_mapping.pyandllm_attribution.py, adding a row toCATEGORY_TO_KIND,.get("model")/"model" inthree times, and the relocation commits of [Relocate] Kernel agent: move the kernel tools to their owner packages and retire HYPERLOOM_KERNEL_AGENT_ROOT #1787 and [Relocate] Coordinator layer: move code to the collaborator that owns it #1794 raise no comment or literal refusal ([Relocate] Kernel agent: move the kernel tools to their owner packages and retire HYPERLOOM_KERNEL_AGENT_ROOT #1787 keeps one true positive: a third identical package string).Size/complexity triggers crossed: none; the gate measures its own scripts and they have no baseline entries.
If this simplifies or refactors: the nested-JSON baseline becomes one line per entry; the maintainability index and the pylint count dimensions, with radon, are removed.
Observable effect: every PR gets the
code-metricscheck and one sticky report comment. A PR fails it when a file it touches gains a unit over a limit or makes a recorded one worse, leaves a fixed unit in the baseline, grows the baseline without thebaseline-raiselabel, adds a long comment block or history in a comment, adds CJK text (files, PR title, body or commits), imports test code from production code, or adds a third copy of a literal, or adds code that reaches into another package's private names, branches on a flag argument, returns a 3+-tuple, compares a raw enum value, writes a global, only forwards its parameters, or declares a single-implementation abstraction. Violations in files the PR does not touch no longer fail it.Breaking changes: no (CI only; the checks are not required until an admin marks them)
PR addresses single concern: no: it bundles the four CI-gate PRs on request, so they land together and the review covers one gate set.
Root cause is upstream: n/a
🤖 Generated with Claude Code