docs(review-pr): add the A rule family for code shape, and the AGENTS.md principles it rests on

[zoroyihan7]

• Description: what and why

  This PR reuses docs(review-pr): add the A rule family for code shape, and the AGENTS.md principles it rests on #1815 and replaces the content of branch ci/ruff-principle-rules. The earlier ruff-rules content of docs(review-pr): add the A rule family for code shape, and the AGENTS.md principles it rests on #1815 now lives in ci: code-metrics gate on touched files with baseline-free checks; fold in import-linter, PR hygiene and ruff principle rules #1812; nothing from it remains on this branch.

  Reviews of this repository keep returning to the same code-shape questions -- private names used across packages, per-framework switches repeated at each consumer, flag arguments, positional tuples and dict bags as results, query-named functions with side effects, single-implementation abstractions, forwarding layers. Until now those findings had no rule to point at, and most could not be blocking because no AGENTS.md sentence stated them. This PR writes them down in two places:

  1. AGENTS.md gains a Code shape bullet (16 lines) beside Clean design, Trust the caller and Simplify by removing a mechanism. It states the principles; it does not change any existing text, including the Size is a design signal complexity paragraph. Where a principle has a legitimate exception the rules rely on, the sentence says so: a query-named function changes nothing while a command may return its outcome, and an abstraction waits for its second implementation unless a lower layer declares it for a higher one to implement or out-of-tree code implements it.

  2. .claude/skills/review-pr/rules.md gains the A -- Abstraction, boundaries and shape family, A1-A14, in the house format, with index rows so a reviewer reaches them from the diff. Every rule keeps at least one Seen in re-read on the merge base (880c167); citations that did not hold were corrected or dropped. Each rule's Severity names the AGENTS.md sentence it rests on. Severity follows a calibration replay (see Calibration below): a rule or a clearly delimited sub-case is blocking when it measured precision >= 0.8 on at least two real findings and rests on an AGENTS.md sentence that states it outright; everything else is advisory:

     Rule	Fires	Real	Precision	Severity
     A1 cross a module boundary only through exported names	10	5	0.50	blocking only for an added non-test import of an _-name or through an _ module segment (5/5; Code shape: "an _-name or an _internal package is not an interface"); demoted to advisory otherwise
     A2 dependencies point one way down the layers	2	2	1.00	blocking (Clean design: "dependencies pointing one way down the layers — no cycles")
     A3 utility vs business module placement	1	1	1.00	advisory (below two real)
     A4 ask the owner, no collaborator navigation	1	0	0.00	advisory
     A5 one reason to change per unit	1	0	0.00	advisory
     A6 extract a function by purpose	0	0	-	advisory (no fire)
     A7 variation point on the owner's spec	1	1	1.00	advisory (below two real)
     A8 parameters	1	1	1.00	blocking for (a), a flag argument that selects between two bodies and makes a caller pass dummies (no fire; keeps its first-landing severity; Code shape: "no flag argument that selects between two bodies"); advisory otherwise (below two real)
     A9 one named result shape	1	0	0.00	advisory
     A10 closed vocabulary typed where it enters	3	1	0.33	advisory
     A11 query names change nothing	1	0	0.00	advisory
     A12 no abstraction before its second implementation, no forwarding layer	2	2	1.00	blocking for a pure forwarding def (Simplify by removing a mechanism: "a forwarding layer with no job of its own"); advisory for single-implementation abstractions (no fire)
     A13 no ceremony the types rule out	2	2	1.00	blocking for a redundant re-check or identity re-conversion (Trust the caller: "redundant re-checks, layered fallbacks, and belt-and-braces defaults buy nothing"); advisory for a pure rename and except: raise (no fire)
     A14 names state what the code does	0	0	-	advisory (no fire)

     No A shape is mechanized yet: the code-metrics job in ci: code-metrics gate on touched files with baseline-free checks; fold in import-linter, PR hygiene and ruff principle rules #1812 (head 0962cff) checks none of them, so every A rule stays a review-time check.

     Wrong behaviour that a shape defect causes stays blocking under the C or S rule it breaks.

  Three items fold into existing rules rather than becoming A rules: X2 now treats a refactor title as a claim that nothing observable changed, with a refactor commit subject alone under an accurate title and description advisory (feat(keep): --max-latency-ms as a constraint on every promotion #1297); D1 covers a second code shape for one concern ([Bad smell] refactor: give misleading names the meaning their code has #1658), resting on Finish the replacement and advisory like the A rules; D10 names the hyperloom.common helpers whose private copies fire it (GEMM lane: connect the three unwired shape inputs, and fix the five defects behind them #1348, Move kernel selection from Hyperloom into KernelForge #1408).

  SKILL.md's rule count follows (69 rules, 10 families; the file stays at its 350-line budget), and .github/copilot-instructions.md gains a Code shape review bullet pointing at the new principles.

  Overlaps are assigned rather than reported twice: a private copy of a hyperloom.common helper is D10's, an empty base class or pure forwarder is A12's, a per-framework/backend/arch branch is A7's, a query name on a function with an effect is A11's (unless the effect is a second result channel, which is A9's), and a business rule in a utility module is A3's. A2 states the layer direction from the imports the tree already has and does not depend on any import-linter configuration, so it reads the same before and after the gates in ci: code-metrics gate on touched files with baseline-free checks; fold in import-linter, PR hygiene and ruff principle rules #1812 land.

  No new gate is added here; the rules are review-time checks.

• Calibration

  A1-A14 were replayed on 30 recent merged PRs (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, in 10 batches of 3) and every fire was judged by an independent adversarial judge; contested cases defaulted to false. 26 fires, 15 real (precision 0.58 overall). The per-rule counts are in the severity table above. Every false positive got a precise Not a finding when clause (or a narrower Fires when) so the replayed false positives no longer fire, without dropping any real finding:

  • A1 (5 false positives): tests, conftests and fixtures under any tests/ dir (own, layer-level, or a sibling layer's dir hosting the owner's unit tests) that import, call, assert on or monkeypatch the private name under test (fix(agentx): derive explore winner axes, park baseline aiter drops off NFS, sandbox aiter in tests #1806 x3, fix(gemm-tune): keep the TunableOp recording envs in the vLLM shape capture #1772); an unchanged _ access whose module load alone changed from sys.path to a package import ([Relocate] Kernel agent: move the kernel tools to their owner packages and retire HYPERLOOM_KERNEL_AGENT_ROOT #1787, V1).
  • A4: aggregate root to a held sub-record's public method, already addressed that way in the file (fix(enablement): make enablement_setting.sh reproduce the setup on its own #1789).
  • A5: moving one single-job collaborator with the private helpers only it calls ([Relocate] Coordinator layer: move code to the collaborator that owns it #1794).
  • A9: a persisted, replayed predicate-inputs dict with a JSON schema and a phase discriminator, read with .get() for older records (fix(prelude): keep PRELUDE open until the initial roofline books #1779).
  • A10: a reader repeating the owner's normalise-then-compare-to-exported-constant idiom on a field the owner stores as str (fix(orchestrator): keep a PRELUDE skip_to_kernel hint from ending FRAMEWORK_AGENT #1808, fix(prelude): keep PRELUDE open until the initial roofline books #1779); a raw literal compared where the owner exports the constant still fires.
  • A11: a resolve_* slice split out of a function that already wrote the same args.<field> in place on the merge base, the write moving unchanged (V1, fix(cli): export every --extra-env pin so all readers resolve it alike #1797); a new write still fires.

• Linked issue(s): none

• Tests: docs only. Ran codespell 2.4.1 on the changed files (only the two pre-existing unparseable hits in S-family text, unchanged by this PR), reuse lint (compliant), python scripts/check_cli_references.py (pass), git diff --check (clean), and a script confirming every AGENTS.md sentence quoted in an A rule's Severity is present verbatim in AGENTS.md and every A rule is on an index row.

• Size/complexity triggers crossed: none (no code).

• If this simplifies or refactors: n/a.

• Observable effect: none for operators -- docs and review instructions only. Authors and reviewers now have an AGENTS.md principle and a numbered rule (A1-A14) to cite for code-shape findings, and the review skill marks blocking the calibrated cases in the table above (A1 added non-test import, A2, A8(a), A12 pure forwarder, A13 redundant re-check); the rest are adjudicated but not published.

• Breaking changes: no

• PR addresses single concern: yes -- the code-shape principle and the review rules that check it.

• Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed: n/a

🤖 Generated with Claude Code

[zoroyihan7]

The earlier ruff-rules content of this branch now lives in #1812. This PR was reopened and reused for the code-shape review rules (A family) and the AGENTS.md Code shape block.