[napetrov]

Refs #94 (§5 stage F1a). Draft: D10 is still under review. No flag, default or output changes; the numbers are unchanged.

Summary

dokimi/dokimi/cli_flags.py          data only: 23 concepts (§2.4 rows)
  Concept(name, canonical=(Flag,..), banned=(...))
  registry_problems(concepts=CONCEPTS): one case per check in tests/test_cli_flags.py
tests/test_cli_flags.py             ratchet over dokimi.cli.build_parser()
  every option: canonical, or on tests/data/cli_flags_allowlist.txt
  banned synonym (not allowlisted): fails, names the canonical spelling
  allowlist row unused, or now canonical: fails (the list keeps no stale row)
  spellings only: an option's type and action are not compared with the registry's `kind`
  snapshot content: checked on Python 3.13 (see below)
docs/reference/cli-interface.json   #88 surface dump; make cli-snapshot regenerates it
tests/data/cli_flags_allowlist.txt  7 rows added for flags main gained (#172, #217), each with a reason
.github/workflows/ci.yml            typecheck job: "CLI snapshot is current" step (3.13)
CONTRIBUTING.md                     "Adding or renaming a CLI flag" (how to add a flag)
changelog.d/..._test-cli-flag-registry.md   Documentation

Scope is the registry and its checks only. cli_flags.py is read by no command; F1b moves each leaf onto it and tombstones old spellings.

Review round (f309ca5 → this head)

• The docs page docs/contributing-cli-flags.md (the 33rd Markdown file under docs/, past max_docs: 32) is now a section of CONTRIBUTING.md. The page, its mkdocs.yml nav entry and its docs/README.md entry are gone. max_docs is unchanged: docs/**/*.md counts 32.
• intel/main is merged in. Main gained flags the registry did not know: catalog trigger-probe (--claude, --sealed, --listing-budget-fraction, --trials, feat(catalog): trigger-probe in real Claude Code; payload leak audit #217) and novelty routing (--all-vs-all, --include-disabled, --queries-by-skill, feat(novelty): routing takes the catalog field, skips disabled and mirrored skills, adds all-vs-all #172). Each has an allowlist row with its reason. The snapshot is regenerated with make cli-snapshot.
• --trials is a banned synonym of reps. The earlier table here asked for a rename to --reps; this PR does not do it, because renaming a shipped flag changes the CLI. It stays on the allowlist until F1b.
• The banned-synonym probe in test_a_new_banned_synonym_fails_and_names_the_registry_spelling used --trials, which is now allowlisted, so it runs with that row removed.
• The nit from the review (registry_problems untested per check) was fixed in f309ca5 and is pinned by one test per check.

Friction for flag-adding PRs

• Canonical spelling, nothing else: run make cli-snapshot and commit the JSON.
• Non-canonical spelling: one allowlist row with a reason, or rename to the canonical one.
• Banned synonym (not allowlisted): rename.

An allowlist row exempts its spelling on every command, not only the one it was added for. A new command may therefore use an allowlisted banned spelling without failing. F1a accepts this: the check does not stop an allowlisted banned spelling from spreading to new commands. Keying the allowlist on (command, option) would close that gap; it is left for F1b. Adding an allowlist row is a review decision; the test does not check that a new row has a reason.

Open PRs that add flags (#155 is the only one still open; #172 and #217 merged and are now on the allowlist): #155 adds --arm --contract --json --outcomes --pass-k --path-stability --shell --threshold. --outcomes, --contract, --shell, --pass-k and --path-stability need one allowlist row each when #155 rebases onto this branch.

Evidence

Temporary edit to dokimi repo scan (reverted): --probe-flag (not in the registry) and --trials (banned for reps). Excerpt of the failing run (messages wrapped here, otherwise verbatim):

$ pytest tests/test_cli_flags.py -rf
E   AssertionError: dokimi repo scan: `--probe-flag` is neither a canonical spelling in dokimi/dokimi/cli_flags.py
    nor on dokimi/tests/data/cli_flags_allowlist.txt. Use the registry spelling of its concept, or, with a
    reason, add `--probe-flag  # <reason>` to dokimi/tests/data/cli_flags_allowlist.txt. Then run
    `make cli-snapshot` if the CLI surface changed.
E         dokimi repo scan: `--trials` is a banned synonym of `reps`. Use `--reps` (dokimi/dokimi/cli_flags.py).
FAILED tests/test_cli_flags.py::test_every_spelling_on_the_tree_is_canonical_or_allowlisted
2 failed, 9 passed

After merging main and before this round's fixes, the ratchet failed on the seven flags above and the snapshot was stale (2 failed, 9 passed). With the rows added and the snapshot regenerated, the banned-synonym probe still failed (1 failed, 15 passed), because --trials is now allowlisted; the probe now runs without that row. After this round: pytest tests/test_cli_flags.py tests/test_import_architecture.py gives 37 passed.

make ci-local on 393b7a5 (box run, TMPDIR=/tmp/...): every gate passes except two. pip-audit: CVEs against filelock==3.32.7, which main pins; this PR changes no dependency file, so it is inherited. The test step: 2 failed on the first run (AF_UNIX path too long under this box's long scratch path, and an unraisable socket warning), and both pass alone. The full suite on this head, pytest -n auto, gives 9707 passed, 5 skipped.

Merge Danger

Door: two-way. Test, data, docs and CI-step only; no runtime path imports cli_flags. Reverting the commits removes the registry, the snapshot and the typecheck step.

Blast Radius: CI. A PR that changes the CLI surface must regenerate the snapshot (the typecheck step fails it otherwise). Two open PRs that each change the surface can both pass CI against an older main and merge one after the other: main then holds a stale snapshot and the typecheck step goes red until someone regenerates it. Nearby edits to the 13k-line JSON also conflict. Branch protection with "require branches to be up to date" closes the first gap; it is not configured here. The cli-interface workflow (#88) is unchanged and report-only.

Changelog: the fragment is filed under Documentation, the existing category for contributor-facing docs. scriv's configured categories (pyproject.toml) have no Internal. The changelog.d/README.md route for internal changes is the skip-changelog label; a maintainer may prefer that.

🤖 Generated with Claude Code

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.49%. Comparing base (f0f95a2) to head (6be19a2).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #254   +/-   ##
=======================================
  Coverage   93.48%   93.49%           
=======================================
  Files         272      273    +1     
  Lines       35363    35419   +56     
  Branches     6001     6013   +12     
=======================================
+ Hits        33060    33114   +54     
- Misses       1489     1491    +2     
  Partials      814      814           

Files with missing lines	Coverage Δ
dokimi/dokimi/cli_flags.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[antonwolfy]

The registry, the ratchet and the snapshot step hold up, but the PR adds a docs page that pushes the docs benchmark over its cap, so make ci-local fails on the benchmark step. One bug inline (the page that crosses the cap), one nit on the registry self-check test.

What I ran, on a Linux review host at ada2d05:

• make ci-local: 28 passed, 1 failed, 7 skipped. The failure is agent-quality.yml: Run benchmark (below). The new CLI snapshot is current step, mypy, deptry, mkdocs build and the wheel smoke tests pass.
• 9,043 tests passed, 3 skipped; changed-line coverage 90% (cli_flags.py lines 312, 314, 320, 322, 325 are not covered).
• A mutation run over the new test file and registry: 30 mutants, the ratchet and walker mutants are all killed; the five registry_problems checks and the CANONICAL/BANNED tables survive.

Checked, no issues

• Snapshot: the dumper run twice gives byte-identical output, equal to the committed cli-interface.json, with no host paths in it. The CI step's working directory, PYTHONPATH and Python 3.13 match the snapshot's ast.dump hashing.
• Ratchet: every option on build_parser() is canonical or allowlisted; all 194 allowlist rows are used, none duplicated; unused and now-canonical rows fail, as do banned synonyms that are not allowlisted.
• Layering and docs: dokimi.cli_flags sits in the foundation layer and test_import_architecture.py passes; the new page is in mkdocs.yml nav and docs/README.md.

[antonwolfy]

I pushed a commit per nit, up to f309ca5, which fix the nit from my review:

• f309ca5: dokimi/dokimi/cli_flags.py:305: registry_problems takes the concepts as an argument (default CONCEPTS), and dokimi/tests/test_cli_flags.py adds one case per check asserting the message.

make ci-local at f309ca5: 28 passed, 1 failed, 7 skipped.

GitHub CI has not run on these commits: the branch conflicts with main, and Actions runs no workflow on a conflicting PR. CI will check them once main is merged in.

Not approving yet. Still open: the bug from my review.

[vchamarthi]

Review: Needs fixes before merge

Adds a CLI flag registry (concepts, canonical spellings, banned synonyms) and a ratchet contract test over the CLI tree, plus the #88 interface snapshot regenerated. Data only: no command reads the registry yet.

Blocking

• dokimi/benchmarks/spec.v1.yaml:83: this PR adds a 33rd markdown page under docs/ while golden_manifest.max_docs stays at 32, so the benchmark step raises golden_manifest violation: matched 33 document(s) ... more than max_docs=32 on the file's own stated rule (raise max_docs in the same commit that adds the doc). Fix: bump max_docs to 33 in this PR.
• merge conflict with main: this PR cannot merge as is and needs a rebase or merge-conflict resolution against the current main branch, independent of the review findings below.

Earlier review threads

Status	Count	Threads
Fixed, with a test	1	untested registry_problems branches (now parametrized, 5 cases)
Still open	1	docs golden_manifest max_docs vs 33rd doc

CI

Check	Result	Cause
benchmark / Run benchmark	fail	introduced (max_docs, same as the open thread above)
static / schema_compat vs the PR base	fail	pre-existing on the merge base (no schema file in this PR's diff)
test / Run tests	fail	pre-existing (3 failures, none in files this PR touches: two need an optional dependency not installed, one is a flaky timeout)
pip-audit	fail	pre-existing (known CVEs in a transitively pinned package, unrelated to this PR)
everything else (lint, type check, CLI snapshot check, coverage, package/install smoke, docs build, CLI surface diff)	pass

This PR also has a merge conflict against main that must be resolved before it can land.

Lines outside the diff, so listed here:

dokimi/benchmarks/spec.v1.yaml:83: blocking (still open from @antonwolfy): max_docs is still 32 while this PR's new page (docs/contributing-cli-flags.md) brings the matched set to 33.

This line's own comment (above, line 80) says to raise max_docs in the same commit that adds a doc. The benchmark step fails with golden_manifest violation: matched 33 document(s) under .../dokimi, more than max_docs=32. include=['docs/**/*.md'] exclude=['docs/archive/**'].

Suggested fix:

  max_docs: 33

dokimi/tests/test_cli_flags.py:144: Fixed (was raised by @antonwolfy): registry_problems now takes a concepts argument (default CONCEPTS), and test_registry_problems_flags_each_inconsistency exercises each of the 5 inconsistency checks with a two-concept fixture, asserting the exact message. The earlier gap (only the real registry was exercised, so a dropped check still passed) is closed.

[napetrov]

Re the review "Not approving yet" by antonwolfy: the one open item was the docs-page cap. It is fixed in 46bed29: the page is removed, its how-to is a section of dokimi/CONTRIBUTING.md, and docs/**/*.md counts 32 again. main is merged in (fee540d). The nit is already fixed in f309ca5.

Declined: raising max_docs. The cap is what makes a new doc a deliberate choice.

[napetrov]

Reply to the "Needs fixes" review, on head 393b7a5.

Blocking: max_docs 32 vs the 33rd doc. Fixed in 46bed29; max_docs is not raised. The page, its mkdocs.yml nav entry and its docs/README.md entry are removed, and the how-to is the section "Adding or renaming a CLI flag" in dokimi/CONTRIBUTING.md. On head, select_docs over benchmarks/spec.v1.yaml matches 32 documents against max_docs: 32. Raising it to 33 is declined: no doc is added, so there is nothing to raise it for, and the cap is what makes a new doc a deliberate choice.

Blocking: merge conflict with main. Resolved. main is merged in (fee540d). Head is MERGEABLE, and a merge of head with current origin/main (114cb6c) is clean.

Earlier thread: untested registry_problems branches. Fixed in f309ca5. registry_problems takes concepts (default CONCEPTS), and test_registry_problems_flags_each_inconsistency has one case per check, each asserting the message. On 393b7a5, tests/test_cli_flags.py and tests/test_import_architecture.py pass (36 passed, 1 skipped).

[antonwolfy]

LGTM. The docs page that crossed max_docs is gone, its how-to is a section of CONTRIBUTING.md, and the golden set is back at 32 documents against max_docs: 32. The new allowlist rows and the regenerated snapshot match the tree after the merge of main.

What I ran, on a Linux review host at 33d8ff2:

• make ci-local: 27 passed, 2 failed, 7 skipped. tests/test_cli_flags.py and tests/test_import_architecture.py pass. pip-audit fails on filelock==3.32.7 in the lock files, which this PR does not touch. Run tests has one failure, tests/test_reference_checks.py::test_the_regex_limit_is_shared_by_every_pattern_of_one_check, a wall-clock bound (4695.04 - 4693.45 < 1.5) that passes when run alone; the PR does not touch that file.
• A mutation run over cli_flags.py and the new test file: 21 mutants. The registry_problems checks, canonical renames and the walker are killed.
• The snapshot dumper on Python 3.13, run on a clean export of the head: byte-identical to the committed cli-interface.json.

Checked, no issues

• Docs cap: docs/**/*.md without docs/archive counts 32; nothing in the tree still names contributing-cli-flags.md.
• Banned-synonym probe: it now runs with the --trials row removed and requires the message to start with dokimi run: , so catalog trigger-probe's own --trials cannot satisfy it.
• Sign-off: every non-merge commit carries a Signed-off-by.

[antonwolfy]

@napetrov merged into the current main (0a63f4e), this PR fails these tests:

• tests/test_cli_flags.py::test_every_spelling_on_the_tree_is_canonical_or_allowlisted
• tests/test_cli_flags.py::test_the_cli_interface_snapshot_is_current

AssertionError: dokimi context consistency: `--outcomes` is neither a canonical spelling in dokimi/dokimi/cli_flags.py nor on dokimi/tests/data/cli_flags_allowlist.txt. Use the registry spelling of its concept, or, with a reason, add `--outcomes  # <reason>` to dokimi/tests/data/cli_flags_allowlist.txt. Then run `make cli-snapshot` if the CLI surface changed.
dokimi context consistency: `--contract` is neither a canonical spelling in dokimi/dokimi/cli_flags.py nor on dokimi/tests/data/cli_flags_allowlist.txt. Use the registry spelling of its concept, or, with a reason, add `--contract  # <reason>` to dokimi/tests/data/cli_flags_allowlist.txt. Then run `make cli-snapshot` if the CLI surface changed.
dokimi context consistency: `--shell` is neither a canonical spelling in dokimi/dokimi/cli_flags.py nor on dokimi/tests/data/cli_flags_allowlist.txt. Use the registry spelling of its concept, or, with a reason, add `--shell  # <reason>` to dokimi/tests/data/cli_flags_allowlist.txt. Then run `make cli-snapshot` if the CLI surface changed.
AssertionError: dokimi/docs/reference/cli-interface.json is stale: run `make cli-snapshot` in dokimi/ and commit the result
assert {'path': 'dok...d': True, ...} == {'allow_abbre...d': True, ...}

The PR's checks ran before main moved to 0a63f4e, so they did not see it. Merging main into the branch reproduces it; once that is fixed I can merge.

Dismissing until the failure above is fixed: merged into the current main (0a63f4e) this PR fails 2 test(s). I'll review again after that.

[vchamarthi]

✅ OpenCodeReview: Review complete: 0 finding(s) across 8 selected item(s).

Review details

• Commit reviewed: 6be19a2 against merge base f0f95a2
• Coverage: 8 files in 2 groups
• Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
• Usage: 745k tokens (406k from cache), 36s
• Tool calls: 16 (9 code_search, 4 file_read, 3 file_find)
• CLI flag registry implementation, reference snapshot, and tests: 4 files
• CI, Makefile, docs and changelog for CLI flag registry: 4 files

[napetrov]

Re your first review (the docs page over max_docs, and the registry_problems nit). The page is removed in 46bed29, its how-to is a section of dokimi/CONTRIBUTING.md, and max_docs is not raised. The nit is fixed in f309ca5 (yours), pinned by test_registry_problems_flags_each_inconsistency.

[napetrov]

Re "Not approving yet; still open: the bug from my review". That bug is the docs page over max_docs, fixed in 46bed29 (page removed, cap unchanged); your nit commit f309ca5 is already in the branch.

[napetrov]

Re "Needs fixes before merge". (a) max_docs: 33: not applied, because the 33rd page no longer exists (removed in 46bed29; the how-to lives in dokimi/CONTRIBUTING.md), so the spec is untouched. (b) Merge conflict: main is merged (1c60eb7), and merged again now at f0f95a2 (44f1959), clean. The CI rows you list as pre-existing are not touched here.

[napetrov]

Thanks. Nothing in this review asks for a change. The snapshot and allowlist state after main moved again is handled in the answer to your later comment (IC_kwDOVBg7g88AAAABgzGjEw).

[napetrov]

Codecov report (IC_kwDOVBg7g88AAAABf5EGHA): it reports no problem, so no change.

[napetrov]

Reproduced after merging main (f0f95a2, newer than the 0a63f4e you named): both tests were red, with seven unregistered spellings (--outcomes, --contract, --shell, --pass-k, --path-stability, --threads, --prompt-file) and a stale snapshot. Fixed in c9c36e0: one allowlist row per spelling with its reason, and make cli-snapshot regenerated. tests/test_cli_flags.py and tests/test_import_architecture.py pass (37 passed).

[napetrov]

OpenCodeReview summary (IC_kwDOVBg7g88AAAABg1-kkg): 0 findings, so no change.