Skip to content

docs(layering): kill criteria on every rule module - #2244

Merged
thymikee merged 1 commit into
mainfrom
claude/layering-kill-criteria
Sep 3, 2026
Merged

docs(layering): kill criteria on every rule module#2244
thymikee merged 1 commit into
mainfrom
claude/layering-kill-criteria

Conversation

@thymikee

@thymikee thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

Every layering rule/policy module under scripts/layering/ now carries a four-line header —
Catches:, Evidence:, Cost:, Kill criterion: — stating what defect class the rule catches
and why no other gate sees it, a real PR/commit that motivated or exercised the rule, its LOC
cost (rule + test), and the concrete condition under which the rule gets deleted. This covers R2,
R4-R7, R9-R14, R16, R18, R19, R65-R73, and the rule-id uniqueness gate (rule-ids.ts). Modules that
share one file (daemon-modularity.ts for R9/R10; check.ts for R4/R5/R6) get one header per rule.
No behavior change, no code lines touched — comment-only.

R3 platforms-seam has no active declaration to annotate (zone-policy.test.ts asserts its
absence); R17 devices and R98/R99 are not declared anywhere under scripts/layering/ (R17 lives
outside this directory, R98/R99 are collision-test fixtures only) — see Tradeoffs.

Validation

  • pnpm check:layering — green, 198/198 node:test assertions pass.
  • pnpm check:quick (oxlint + tsc -b) — clean.
  • npx vitest run --project unit-core scripts/layering — reports "No test files found": the
    layering suite runs under node --test via check:layering, not vitest's unit-core project,
    which doesn't include scripts/layering/** in its include globs (confirmed pre-existing).
  • Planted-red: added a quoted 'R2 imposter-rule' string inside zone-policy.ts's new header
    comment (the exact mistake the task's CAUTION warns against), then ran pnpm check:layering.
    Observed failure:
    ✖ the layering rules currently in tree carry no unallowed collision
    AssertionError: Expected values to be strictly deep-equal: + ['two rules answer to one id: R2 names commands-floor and imposter-rule. Allocate the next free number.'] - []. Reverted; gate
    is green again.
  • Full affected gate: green at 8fdb1c8 (pnpm check:affected --run;
    gates run: format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package,
    integration-node, macos-coverage, vitest-related, integration-progress, replay-compat,
    daemon-wire-compat, affected-selector, gate-manifest, gate-manifest-model, depgraph,
    tmpdir-leaks, tmpdir-leaks-model, coverage-model, wire-compat-model, production-exports,
    bundle-owner-files, fixture-cache, fixture-fallback, command-docs, agent-guidance,
    xctest-selection, maestro-conformance, mutation-model; rest GitHub-authoritative/parked).

Tradeoffs / follow-ups

  • R3 and R17 have no module under scripts/layering/ to annotate today (R3 is retired with only
    a negative test; R17 "devices" lives outside this directory per rule-ids.ts's own history
    comment). R98/R99 only appear as example ids inside rule-ids.test.ts's collision fixtures, not
    as declared production rules — rule-ids.ts itself got a header instead, describing the
    uniqueness gate it implements.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.58 MB 2.58 MB 0 B
JS gzip 869.6 kB 869.6 kB 0 B
npm bundled raw 2.58 MB 2.58 MB 0 B
npm bundled gzip 869.6 kB 869.6 kB 0 B
npm tarball 1.02 MB 1.02 MB +14 B
npm unpacked 3.46 MB 3.46 MB 0 B
npm clean-installed 3.46 MB 3.46 MB 0 B

npm unpacked components

Component Base Current Diff
JS / dist source 2.74 MB 2.74 MB 0 B
Apple runner source/project 544.4 kB 544.4 kB 0 B
Apple snapshot presentation source 33.8 kB 33.8 kB 0 B
macOS helper source 54.8 kB 54.8 kB 0 B
Android helper artifacts 43.3 kB 43.3 kB 0 B
Other package files 46.3 kB 46.3 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 31.3 ms 31.2 ms -0.2 ms
CLI --help 83.9 ms 82.0 ms -1.9 ms

Top changed chunks: no changes in the largest emitted chunks.

Top changed packed files

No changed packed files.

@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

BLOCKED — several new kill criteria claim replacement enforcement that does not exist:

  1. [P1] R18/R70 cannot be retired based on package manifests. Node built-ins are not package.json dependencies, globals need no import, and those rules also protect additional mechanics/construction and retired-root invariants. A zero dependency constraint would leave the violations legal. Name complete enforced replacement boundaries or retain the AST gates.

  2. [P1] R72/R73 are not replaced by exports maps. Exports maps restrict external package specifiers, not internal relative calls, required engine→presentation topology, or provider-local residue construction/discard/reassignment. State structural replacements for each invariant or retain these gates.

The same overclaim appears in R11/R13/R16: project references, publishing, or typed ports do not by themselves make relative tunnels, eager imports, or Node/global mechanics unresolvable. Audit every kill criterion to require either an actually enforced replacement or an explicit decision that the invariant no longer matters. Exact-head CI is green; comments only, but the guidance is not safe to merge as written.

@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Updated exact-head status: still blocked at 92bd3dc on the previously reported unsafe kill-criterion guidance, and now also blocked by a current-main conflict in scripts/layering/check.ts after #2240. Rebase, preserve main's R4/R5 commentary, correct the challenged guidance, and rerun affected exact-head checks. Existing green checks predate the rebase.

@thymikee
thymikee force-pushed the claude/layering-kill-criteria branch from 92bd3dc to 447e7cf Compare September 2, 2026 20:33
@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Rebased onto main at d04e213: main's R4/R5/R6 commentary in check.ts kept verbatim (including #2241's "sole owner of the ratchet" note), R14/R71 headers carried into retired-paths-policy.ts, headers for the two deleted modules dropped.

Every kill criterion re-audited against the A4 spike (project references are a build cache, not a boundary). None names an enforced full replacement, so all 25 now read "none enforced today; retire only by maintainer decision that no longer matters": R2, R4, R5, R6, R7, R9, R10, R11, R12, R13 (x3), R14/R71, R16 (x2), R18, R19, R65-R70, R72, R73, rule-ids. Partial mechanisms are named as partial only: R11 cites tsc TS2307 (NodeNext exports) and TS6059 (composite rootDir) for two of four branches; R18/R70 state manifests do not govern built-ins or globals; R72/R73 state exports maps do not restrict relative calls or residue handling.

Reran check:layering (198/198, guard OK), format:check, check:quick. Head: 447e7cf.

@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

BLOCKED at 447e7cf1917d250801d2d74ff6c02ec647cfc6dd.

No remaining code/documentation finding. The unsafe replacement kill criteria are corrected, the check.ts conflict is resolved, and the headers now document structural invariants rather than excuse workarounds.

The only blocker is required iOS Smoke CI: TEXT_INPUT_COMMIT_NOT_OBSERVED while filling the fixture email field. This is unrelated to the comments-only layering diff and strongly consistent with a runner/device flake, but this exact head still needs a successful rerun or explicit baseline control before readiness. All other exact-head checks are green and package size is unchanged. Do not label ready while required CI is red.

Adds a four-line Catches/Evidence/Cost/Kill-criterion header to every
layering rule module for R2, R4-R7, R9-R14, R16, R18, R19, R65-R73,
and the rule-id uniqueness gate, so each structural check states what
it catches, why no other gate sees it, its LOC cost, and the concrete
condition under which it gets deleted. No behavior change.
@thymikee
thymikee force-pushed the claude/layering-kill-criteria branch from 447e7cf to 8fdb1c8 Compare September 3, 2026 06:40
@thymikee

thymikee commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

READY at 8fdb1c81bed391cbdf3231160565d7da0d849457.

Prior unsafe kill-criterion claims remain corrected and check.ts ownership commentary is preserved. The comments-only 23-file layering diff has no remaining finding. Exact-head merge state is CLEAN; all checks, including rerun Coverage and device smoke, are green. Package size is unchanged.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 3, 2026
@thymikee
thymikee merged commit a5a7f6d into main Sep 3, 2026
23 of 24 checks passed
@thymikee
thymikee deleted the claude/layering-kill-criteria branch September 3, 2026 09:13
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-03 09:13 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant