Skip to content

refactor(cli): fold cli-schema into commands/schema, move batch-steps into commands/batch (#2679) - #2678

Merged
thymikee merged 2 commits into
mainfrom
claude/agent-device-adversarial-review-rnu7kf
Sep 19, 2026
Merged

thymikee merged 2 commits into
mainfrom
claude/agent-device-adversarial-review-rnu7kf

Conversation

@thymikee

@thymikee thymikee commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Summary

Implements the move tracked in #2679 ("PR A" from the collocation plan in #2677 (comment)) — filed as its own issue per this scope note: #2677's guard 9 keeps that issue tooling-only, so the src/-touching moves it inspired belong outside it.

  • cli-schema was a standalone 8-file zone with Q ≈ 0, out/in flow 30, touching 29 of 39 families — the plan's clearest "wrongly-a-family" candidate. It exists only to render the command facets commands/ already owns (refactor(cli-schema): orient the schema layer below commands #2543 declared that direction), so it folds into commands/schema/.
  • src/cli/batch-steps.ts's only relative import and only real importer already live under src/commands/, so it moves to src/commands/batch/batch-steps.ts in the same PR per the plan's move-4 bundle.
  • No behavior change. The layering model's targetDagZone classifies zones by top-level src/<folder>/ name, so the moved files automatically re-zone from the retired cli-schema zone into commands — no new zone-policy needed.

Rebase status (see #2679 for detail)

The scope-note comment asked this move to rebase onto apex/module-shape-measures (the #2677 tooling branch) so before/after pnpm coupling/pnpm legibility numbers are attributable to the move alone. I tried it: apex/module-shape-measures (86741f2) is 4 commits behind main and is missing #2665, which also touches src/cli/parser/command-suggestions.ts. Rebasing onto it as-is conflicts there for real (apex has no suggestFlagFor/flag-suggestion code at all, so the conflict can't be resolved by picking a side without either dropping this move's fix for that file or pulling in unrelated feature work). This PR stays based on main for now, green and ready; I'll rebase it onto apex/module-shape-measures and add the coupling numbers once that branch is refreshed against current main.

Changes

  • git mv src/cli-schema/* (8 production + 8 colocated test files) → src/commands/schema/*
  • git mv src/cli/batch-steps.ts → src/commands/batch/batch-steps.ts
  • Updated every import site (~35 files), the literal 'src/cli-schema/command-overrides.ts' owner-files path in src/cli/command-explain.ts (and its test), and a self-referencing import.meta.url path in the moved command-schema-guards.test.ts
  • Dropped the now-absorbed ['cli-schema', 3] entry from scripts/layering/model.ts's TARGET_DAG_RANK, and swapped a hardcoded cli-schema zone out of a model.test.ts fixture (for mcp, another rank-3 zone, preserving the same back-edge-detection shape)
  • Renamed the two src/cli-schema/...-keyed entries in fallow-baselines/health.json
  • Updated the AGENTS.md and docs/agents/cli-flags.md pointer lines

Left deliberately unchanged (dead/inert but harmless, not required for correctness): scripts/layering/zone-policy.ts/.test.ts and scripts/layering/daemon-modularity.ts/.test.ts still name cli-schema in a policy rule that will simply never fire again (the daemon -> commands R2 rule already covers the same boundary); a comment in scripts/layering/check.ts and scripts/help-conformance-cases.mjs; two historical/append-only docs (docs/dependency-graph-findings.md, docs/adr/0019-...).

Verification

  • pnpm lint, pnpm typecheck, pnpm format:check, pnpm check:layering (244/244), pnpm depgraph:test (24/24), pnpm check:gate-manifest — all clean
  • pnpm test:unit (10,522 tests) — 3 pre-existing failures in packages/capture-kit and src/daemon, confirmed to reproduce identically on origin/main (unrelated to this change, verified via a throwaway worktree)
  • Targeted vitest run over all 16 moved test files plus every import-touched file: all pass
  • An independent adversarial review (Fable) recomputed every changed import path against the new file locations, searched for missed references repo-wide (including hidden config), checked for directory-name collisions, and re-verified the layering/fallow-baseline changes — reported clean, no defects found

Test plan

  • pnpm check:layering
  • pnpm lint / pnpm typecheck / pnpm format:check
  • pnpm test:unit (pre-existing unrelated failures confirmed on main)
  • Adversarial review pass
  • Rebase onto refreshed apex/module-shape-measures + pnpm coupling before/after (blocked, see above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01EmJYA9V4uKcAGBqedwXbwD

… into commands/batch

Implements move 1 (+ batch-steps.ts) from the collocation plan in #2677:
cli-schema was a rank-0.0003-modularity, 37.5%-legible eight-file zone that
existed only to render the command facets commands/ already owns, and
batch-steps.ts's only relative import and only importer both already live
under commands/. Folding both in removes a family the coupling/legibility
measures called out as a clear placement miss, without changing behaviour.

- git mv src/cli-schema/* -> src/commands/schema/*
- git mv src/cli/batch-steps.ts -> src/commands/batch/batch-steps.ts
- update every import site, the literal command-explain.ts owner-files
  path, the layering model's now-absorbed cli-schema rank entry (folded
  files resolve to the commands zone by directory), fallow-baselines/
  health.json's path-keyed findings, and the AGENTS.md / cli-flags.md
  pointers

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EmJYA9V4uKcAGBqedwXbwD
@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.62 MB 4.62 MB 0 B
Package (unpacked) 4.62 MB 4.62 MB 0 B
Package (download) 1.37 MB 1.37 MB -6 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.2 ms 26.0 ms -0.2 ms
CLI --help 78.2 ms 75.8 ms -2.4 ms

@thymikee thymikee changed the title refactor(cli): fold cli-schema into commands/schema, move batch-steps into commands/batch refactor(cli): fold cli-schema into commands/schema, move batch-steps into commands/batch (#2679) Sep 19, 2026

Copy link
Copy Markdown
Member Author

Measured before/after, and one enforcement gap

The rebase is not blocked. Merging origin/main into apex/module-shape-measures first (no conflicts), then merging this PR (no conflicts), gives a tree where pnpm coupling runs. The command-suggestions.ts conflict only appears when rebasing onto the tooling branch's stale base, which predates #2665. Refresh the tooling branch against main, or merge instead of rebasing.

pnpm coupling from the tooling branch, before = tooling + main (9ca85d21), after = + this PR (a6f5ceec):

window cross-family Q ≥ 5-family share
all-time 64.3 → 62.2% 0.251 → 0.255 37.7 → 35.7%
120 days 62.5 → 60.1% 0.283 → 0.290 39.7 → 37.3%

Families 39 → 38. commands: 132 → 141 files, Q 0.0414 → 0.0460, out/in 2.35 → 2.60. cli: Q 0.0127 → 0.0124 (batch-steps leaving). Matches the plan's what-if (62.4 / 0.255 / 35.9) within rounding. On the merged tree repo-history:test, coupling:test, legibility:test and check:layering all pass.

Enforcement gap. The PR leaves two rules naming a zone that no longer exists and calls them inert. They are not equivalent:

  • checkDaemonCliSchemaBoundary in scripts/layering/daemon-modularity.ts (root src/cli-schema/) is dead, but the daemon side is still covered: R2 row 1 forbids daemon → commands for every import kind, and src/commands/schema/ is inside commands.
  • R2 row 2 in scripts/layering/zone-policy.ts (commands must not import cli-schema, the refactor(cli-schema): orient the schema layer below commands #2543 direction) is now enforced by nothing. It still holds on this head, since no file under src/commands/ outside schema/ imports commands/schema/, but nothing stops the help renderer being pulled back into the command surface later.

AGENTS.md asks to complete migrations and remove superseded paths, and to colocate claims with their enforcement. Suggested fix, small: replace R2 row 2 with a folder-scoped rule (commands outside schema/ must not import src/commands/schema/), delete the dead R10 check and its cases in daemon-modularity.test.ts, retarget the zone-policy.test.ts cases, and update the header comment in scripts/layering/check.ts that still draws { commands, cli-schema }. If the direction is instead considered retired, delete all of it and say so here.


Generated by Claude Code

…ed into it

Review on #2678 found a real enforcement gap: folding cli-schema into
commands/schema/ put both sides of the #2543 "commands never imports
cli-schema back" direction into the same commands zone, which
checkLayeringRules's ZONE_POLICIES table cannot see (it skips every
same-zone edge before the table ever runs). The R2 zone-policy row
that used to declare the direction became silently unenforceable, not
just harmlessly dead like the daemon-side check.

- add scripts/layering/commands-schema-boundary.ts: a folder-scoped
  check (commands outside schema/ must not import commands/schema/)
  wired into check.ts's rule registry, replacing the lost R2 row
- drop the now-unenforceable R2 row from zone-policy.ts's table and
  its test, and the header comments that described it
- delete the actually-dead checkDaemonCliSchemaBoundary from
  daemon-modularity.ts (the daemon side stays covered generically by
  R2's daemon/core -> commands row) and its tests

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EmJYA9V4uKcAGBqedwXbwD

Copy link
Copy Markdown
Member Author

Thanks for the measurement and the catch.

Rebase/measurement: noted — merge instead of rebase avoids the stale-base conflict, and the numbers you posted confirm the move lands as the plan predicted (Q 0.251 → 0.255, cross-family 64.3 → 62.2%, matches the what-if within rounding). Nothing for this PR to change there; it stays based on main.

Enforcement gap: fixed in b03e8cf, going with the small fix rather than retiring the direction:

  • scripts/layering/commands-schema-boundary.ts — a new folder-scoped check (commands outside schema/ must not import commands/schema/), wired into check.ts's rule registry as commands-schema-boundary, replacing the lost R2 row. You were right that this needed a real mechanism, not zone-policy: checkLayeringRules skips every same-zone edge before ZONE_POLICIES ever runs, so the old row was unreachable the moment both sides shared a zone, independent of whether I'd kept the row text.
  • Dropped the now-unenforceable R2 row from zone-policy.ts and its test, and updated the header comments in zone-policy.ts and check.ts (including the { commands, cli-schema } spine diagram you flagged).
  • Deleted checkDaemonCliSchemaBoundary from daemon-modularity.ts and its two tests — confirmed harmless to drop, since R2's daemon/core -> commands row already forbids the daemon from reaching src/commands/schema/ generically (it's inside commands).

pnpm check:layering (246/246, was 244 + 5 new − 2 removed daemon tests − 1 removed zone-policy test), lint, typecheck, format:check all clean; targeted vitest over the touched areas passes.


Generated by Claude Code

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at b03e8cf. No blocking findings: the moves are renames with import-path edits only, no live import of src/cli-schema or the old batch-steps path is left, and the new commands-schema-boundary rule replaces the zone-policy row that same-zone edges had made unreachable, while the core/daemon -> commands row still covers the deleted daemon check. CI is green.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 19, 2026
@thymikee
thymikee merged commit 8bbf7c6 into main Sep 19, 2026
19 checks passed
@thymikee
thymikee deleted the claude/agent-device-adversarial-review-rnu7kf branch September 19, 2026 13:51
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.

2 participants