Repository navigation
test(cli): flag registry, ratchet contract test, CLI snapshot (#94 F1a) - #254
Conversation
… F1a) Data and tests only; no flag, default or output changes. - dokimi/cli_flags.py: each flag concept of intel#94 section 2.4 with its canonical spellings, type and banned synonyms. Nothing reads it yet (F1b moves leaves). - tests/test_cli_flags.py: walks dokimi.cli.build_parser() in ratchet mode. A spelling that is neither canonical nor allowlisted fails, a banned synonym fails with the canonical spelling named, and allowlist rows that are unused or already canonical fail, so the allowlist only shrinks. An allowlist row exempts its spelling on every command. - tests/data/cli_flags_allowlist.txt: today's non-canonical spellings, one reason per row where it is not obvious. - docs/reference/cli-interface.json: the intel#88 surface dump, regenerated by `make cli-snapshot` (venv Python). The content check runs on Python 3.13, the pinned version: the dumper hashes validators with ast.dump, which differs by version. CI's typecheck job runs it on every PR (a PR runs no 3.13 test leg); the ratchet and structural checks run on every test leg of the matrix. - tests/test_import_architecture.py: cli_flags sits in the foundation layer. - docs/contributing-cli-flags.md: how to add or rename a flag. - Internal changelog fragment. Refs intel#94 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
antonwolfy
left a comment
There was a problem hiding this comment.
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 isagent-quality.yml: Run benchmark(below). The newCLI snapshot is currentstep,mypy,deptry,mkdocs buildand the wheel smoke tests pass.- 9,043 tests passed, 3 skipped; changed-line coverage 90% (
cli_flags.pylines 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_problemschecks and theCANONICAL/BANNEDtables 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,PYTHONPATHand Python 3.13 match the snapshot'sast.dumphashing. - 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_flagssits in the foundation layer andtest_import_architecture.pypasses; the new page is inmkdocs.ymlnav anddocs/README.md.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to f309ca5, which fix the nit from my review:
f309ca5:dokimi/dokimi/cli_flags.py:305:registry_problemstakes the concepts as an argument (defaultCONCEPTS), anddokimi/tests/test_cli_flags.pyadds 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.
…flag-registry Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/tests/test_import_architecture.py
The new page made the 33rd Markdown file under docs/, past the docs benchmark's max_docs of 32. The how-to now lives in a section of CONTRIBUTING.md under "Adding things"; the page, its nav entry and its docs/README.md entry are removed. max_docs is unchanged. Thread: PRRT_kwDOVBg7g86xE1Df Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…obe strict Main now has flags the registry does not know: catalog trigger-probe's --claude, --sealed, --listing-budget-fraction and --trials (intel#217), and novelty routing's --all-vs-all, --include-disabled and --queries-by-skill (intel#172). Each gets an allowlist row with its reason. --trials is a banned synonym of reps; renaming it would change a shipped command, so it waits for the F1b migration. The banned-synonym probe in test_cli_flags.py used --trials, which is now allowlisted, so the probe would pass for the wrong reason. It runs with that row removed instead. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Written by make cli-snapshot on the pinned interpreter; no hand edits. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
There is no Internal category in scriv's configuration; the fragment now uses Documentation, the existing category for contributor-facing docs. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
f309ca5 makes registry_problems take the concepts and adds one case per check (tests/test_cli_flags.py), so each of the five checks is shown to fail. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
catalog trigger-probe's own --trials produces the same message, so the probe passed whether or not the injected dokimi run leaf was walked. The assertion now requires the message to name dokimi run. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
vchamarthi
left a comment
There was a problem hiding this comment.
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 underdocs/whilegolden_manifest.max_docsstays at 32, so the benchmark step raisesgolden_manifest violation: matched 33 document(s) ... more than max_docs=32on the file's own stated rule (raise max_docs in the same commit that adds the doc). Fix: bumpmax_docsto 33 in this PR.- merge conflict with
main: this PR cannot merge as is and needs a rebase or merge-conflict resolution against the currentmainbranch, 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.
|
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 Declined: raising |
|
Reply to the "Needs fixes" review, on head 393b7a5. Blocking: Blocking: merge conflict with Earlier thread: untested |
…flag-registry Signed-off-by: Petrov, Nikolay A <nikolay.a.petrov@intel.com>
The nit this review reports (registry_problems takes the concepts) is in f309ca5. Signed-off-by: Petrov, Nikolay A <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
There was a problem hiding this comment.
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.pyandtests/test_import_architecture.pypass.pip-auditfails onfilelock==3.32.7in the lock files, which this PR does not touch.Run testshas 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.pyand the new test file: 21 mutants. Theregistry_problemschecks, 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/**/*.mdwithoutdocs/archivecounts 32; nothing in the tree still namescontributing-cli-flags.md. - Banned-synonym probe: it now runs with the
--trialsrow removed and requires the message to start withdokimi run:, so catalog trigger-probe's own--trialscannot satisfy it. - Sign-off: every non-merge commit carries a
Signed-off-by.
|
@napetrov merged into the current
The PR's checks ran before |
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.
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 8 selected item(s). Review details
|
…apshot After main moved to 0a63f4e and then f0f95a2, the ratchet saw seven spellings that are neither canonical nor allowlisted (--outcomes, --contract, --shell of context consistency; --pass-k, --path-stability of repo compare; --threads, --prompt-file of repo tasks replay), and the committed cli-interface.json was stale. Add one allowlist row per spelling with its reason and run `make cli-snapshot`. Both tests were red on the merged tree and are green. Thread: IC_kwDOVBg7g88AAAABgzGjEw Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Re "Needs fixes before merge". (a) |
|
Thanks. Nothing in this review asks for a change. The snapshot and allowlist state after |
|
Codecov report (IC_kwDOVBg7g88AAAABf5EGHA): it reports no problem, so no change. |
|
Reproduced after merging |
|
OpenCodeReview summary (IC_kwDOVBg7g88AAAABg1-kkg): 0 findings, so no change. |
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 4 selected item(s). Review details
|
antonwolfy
left a comment
There was a problem hiding this comment.
LGTM. The docs page that crossed max_docs is gone, the seven flags that main gained are allowlisted with a reason each, and the regenerated snapshot matches the tree.
What I ran, on a Linux review host at 6be19a2:
make ci-local: 28 passed, 1 failed, 7 skipped.Run testspasses (10,534 passed, 4 skipped), as doCoverage of changed lines, theCLI snapshot is currentstep,mkdocs buildand the wheel smoke tests.pip-auditfails onfilelock==3.32.7in the lock files, which this PR does not touch.- A mutation run over
cli_flags.pyand the new test file: 21 mutants. Theregistry_problemschecks, canonical renames in use and the walker are killed. - The snapshot dumper on Python 3.13: byte-identical to the committed
cli-interface.json, and all seven new flags are in it.
Checked, no issues
- Allowlist rows:
--contract,--outcomes,--shellappear only ondokimi context consistency;--pass-k,--path-stabilityonly ondokimi repo compare;--prompt-file,--threadsonly ondokimi repo tasks replay. All are used, and none is canonical or banned. - Makefile, CI and CONTRIBUTING: the
cli-snapshottarget, the 3.13typecheckstep and the "Adding or renaming a CLI flag" section agree. - Sign-off: all 12 non-merge commits carry a
Signed-off-by.
The merge of main brought intel#254's flag registry and its snapshot check: docs/reference/cli-interface.json lacked the --task option this branch adds to `repo compare`, so test_the_cli_interface_snapshot_is_current failed. Regenerated with scripts/cli_surface.py dump (`make cli-snapshot`). --task needs no allowlist row: it is already a canonical spelling of the registry's `inputs` concept ("a filter on --tasks", text, repeatable), which is what the compare option is (an append of task ids). intel#298's check_comment_blocks.py --base origin/main --head HEAD finds no comment block over two lines among the lines this branch adds or edits, so no comment changes. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
The merge with main brings the flag registry of intel#254: its ratchet test fails a spelling that is neither canonical in dokimi/cli_flags.py nor on tests/data/cli_flags_allowlist.txt, and CI fails a stale docs/reference/cli-interface.json. `--hits` switches repo report from the table to the list of credited replay hits; no registry concept names a view switch, so it gets an allowlist row with that reason, and the snapshot is regenerated with what `make cli-snapshot` runs (the only change is the --hits option). intel#298's scripts/check_comment_blocks.py --base origin/main --head HEAD finds no comment block over two lines among the lines this PR adds or edits, so no comment changes. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…gs inconclusive rows (#296) * feat(repo): compare records start context, filters tasks, flags rows An evaluation of AGENTS.md guidance files (26 review_replay tasks, two arms, three reps) showed three gaps in reading a batch with repo compare. Start context: a batch did not say what each cell started with, so whether an arm's guidance was delivered had to be read by hand from every agent.stdout. The Transcript now carries first_prompt_tokens, the input + cache_creation + cache_read tokens of the first main-thread call, read with dokimi.context.trace (one call per message id, subagent calls apart). repo run writes it as result.json agent.first_prompt_tokens (null until set, and for a trace without such a call), and repo compare compares it as a descriptive measure next to cost_usd and turns. Like a process count, it is compared only when both arms report it, so a batch written before the field has no row rather than a row of zeros. Task selection: repo compare --task ID (repeatable) compares only those task ids. An id no finished cell has is refused through CompareError; the tasks left out are not listed as unpaired nor their cells as missing; the summary records the selection as task_filter and the text header prints "tasks: N of M (--task)". Reading notes, with no number changed: a gate or score row of 2+ tasks whose interval includes 0 gets an "inconclusive" note with mde, the smallest effect 80% power detects at that spread, (1.959964 + 0.841621) * sd / sqrt(n) (mde is in every row of --json); a row whose interval and sign-flip p disagree gets interval_p_disagree and a note naming the two methods, saying for gate and score that p is the one to read; descriptive rows with p < 0.05 are named on one line as uncorrected and outside Holm's family. Contract sections 4 and 9, docs/context-window.md and the compare module docstring describe the field, the flag and the notes. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * test(repo): pin the one-arm skip both ways and the usage merge Review comment 4415626666: the one-arm skip of first_prompt_tokens was tested only with the baseline lacking it. The test now also compares a batch whose treatment lacks it and asserts the same warning and no unpaired entries, so a skip that checks only the baseline arm fails. Review comment 4416413441: the main call's message id now has a third, sparser usage line after the full one, so the first prompt is 19_380 only when the lines merge by maximum; "last line wins" gives 16_319 and fails the test, as "first line wins" already did. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): compare's mde uses the t quantiles of its interval The inconclusive note gave mde with normal quantiles (1.96 + 0.84), but the interval beside it is a t interval at n-1 degrees of freedom. At 4 tasks the note said +-0.639 where the t quantiles give +-0.950; at 26 the gap is small. The contract's formula is updated to match. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * fix(repo): compare --task matches a missing cell by its task, not a prefix A task id may contain '__', so a missing cell of fix__a starts with fix__ and was counted as a cell of fix. The task is now split off the cell name from the right, as the arm, model and rep are the last three parts. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * fix(repo): compare's arm and metric errors say when --task is the reason With --task the arm and metric checks run on the selected cells, so an arm that ran only other tasks read as a wrong name. Those errors now add 'among the --task selection' when a selection is set. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * docs(context-window): say what first_prompt_tokens records The sentence bolded an orphan 'first'. It now names the first prompt. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * docs(repo): what the --task of count counts of is the tasks with a finished cell, the only ids --task can name, not the task count of the batches: a task whose cells all crashed before result.json is not in it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * docs(compare): rewrap the module docstring after the mde change Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * refactor(repo): move compare's --task selection into compare_tasks.py repos/compare.py was 787 lines, and the review answers that follow (4418945747, 4420755924, 4418945753, 4420755898, 4418945763) take it past the 800-line limit of the growth ratchet. The --task selection (_select_tasks, _cell_task) moves unchanged into repos/compare_tasks.py, where the missing-cell fix of 4420755898 adds its code. An unknown id now raises TaskSelectionError there, which compare() re-raises as the same CompareError with the same message. No behaviour changes. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): compare gives no mde where the sign-flip p cannot reach 0.05 Answers review comments 4418945747 and 4420755924. The row's p is an exact sign-flip test, whose smallest p over k changed (non-tied) tasks is 2 / 2**k: at 5 or fewer changed tasks no effect of any size reaches p < 0.05, yet the inconclusive note named an effect that 80% power would detect (gate deltas [1, 1, 0]: p 0.5, mde 1.788; the four-task score row: mde 0.950, p floor 0.125). min_sign_flip_p gives that floor (1 / (N + 1) for the Monte Carlo test above 16 changed tasks). Where it is 0.05 or more, mde is null in every row, and the gate or score note says instead "with 2 changed tasks the sign-flip p cannot go below 0.5: no effect can reach p < 0.05 here". From 6 changed tasks on, mde is computed as before. The module docstring, the contract and the changelog fragment say so, and no longer claim the notes change no number: they change no delta, interval or p. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): compare flags a gate or score row against its Holm p Answers review comment 4418945753. interval_p_disagree was set in paired_summary, before _holm ran, so a gate or score row compared its interval with the raw p while p_holm is what decides it. With two treatments and gate deltas six +1 and one tie, each row printed +0.857 [+0.571, +1.000], p 0.031, p_holm 0.062 and no note. _holm now sets interval_p_disagree on the gate and score rows from Holm's verdict (correct_family, unrounded against 0.05), and their note quotes p_holm and says to read p_holm. Descriptive rows keep the flag on their raw p. The module docstring, the contract and the changelog fragment say so. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): compare --task reads a missing cell's task from its meta.json Answers review comment 4420755898 (and finishes 4418945759). 349d544 took a missing cell's task from its name, split from the right, which assumed the arm and model slugs carry no "__". They can: runner.cell_name keeps the "__" of a model (cell_name("fix", "guidance:raw", "m__v1", 2) is fix__guidance-raw__m__v1__r2, read as task fix__guidance-raw) and arms.slug keeps that of an arm such as skill:lint__v2, so --task fix silently left such an unfinished cell out of the missing list. The task now comes from the cell's meta.json, which repo run writes with the task id when the cell starts, read with the same guarded read as the module's other cell files. A cell only plan.json names (no files) gets the longest task id of its batch's finished cells that its name starts with followed by "__". A cell neither gives a task for is missing only without --task. The name-splitting _cell_task and its comment about slugs are gone. The contract and the changelog fragment say how the task is found. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): compare names the arms --task leaves without a cell Answers the second half of review comment 4418945763. Without --arm, compare takes every other arm that has a finished cell among the --task selection, so an arm that ran none of the selected tasks dropped out of the comparison with no word. task_filter now carries dropped_arms: without --arm, the arms of the batches with no finished cell among the selection ([] with --arm, where such an arm is refused). The text header prints them under the tasks line ("not compared, no finished cell among the --task selection: guidance:raw"). The default arm list is one helper, _other_arms, for both. The module docstring, the contract and the changelog fragment say so; the fragment also says what M of "tasks: N of M" counts (the tasks with a finished cell), which e00edbc said in the contract only (4418945768). To keep repos/compare.py under the 800-line limit, CompareError moves into compare_tasks.py, which now raises it for an unknown --task id directly; compare.py imports it, so dokimi.repos.compare.CompareError is unchanged for its callers. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * test(cli): regenerate the CLI snapshot for repo compare --task The merge of main brought #254's flag registry and its snapshot check: docs/reference/cli-interface.json lacked the --task option this branch adds to `repo compare`, so test_the_cli_interface_snapshot_is_current failed. Regenerated with scripts/cli_surface.py dump (`make cli-snapshot`). --task needs no allowlist row: it is already a canonical spelling of the registry's `inputs` concept ("a filter on --tasks", text, repeatable), which is what the compare option is (an append of task ids). #298's check_comment_blocks.py --base origin/main --head HEAD finds no comment block over two lines among the lines this branch adds or edits, so no comment changes. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): compare gives no mde where Holm keeps p_holm from 0.05 Review comment 4422703568: mde and the inconclusive note read the raw sign-flip floor, but gate and score rows are decided by p_holm. Two score treatments with six changed tasks each both got an mde and an "80% power detects" note, though Holm over the two rows cannot take either p_holm below 2 * 2/64 = 0.0625. After Holm, each gate or score row gets p_holm_floor: its p_holm with its own p put at its floor (min_sign_flip_p over its changed tasks) and the family's other p as observed, from correct_family. Where that is not below 0.05, mde is null and the inconclusive note says "with Holm over N rows, p_holm cannot go below X here: no effect can reach p_holm < 0.05". A family of one row is the raw rule, as before; descriptive rows keep it. min_detectable, ALPHA and POWER move to the new repos/compare_reach.py with the floor helper, holm_floor, so compare.py stays under 800 lines; compare re-imports them. Tests pin the two-treatment case (both rows null, the Holm note) and one where a row of eight changed tasks keeps its mde beside a six-task row that loses it. The module docstring, contract §9, architecture.md and the changelog fragment follow. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * docs(repo): name mde an uncorrected t-based estimate, not Holm power Review comments 4422911586 and 4422911592: `mde` is computed as (t(0.975, n-1) + t(0.80, n-1)) * sd / sqrt(n), the effect a one-sample t test at the uncorrected 5% level detects with 80% power. It is not the power of the sign-flip test nor of the Holm-corrected decision: with two rows and the other row's p at 1, Holm needs this row's raw p below 0.025, and the figure is still the one-row value. `p_holm_floor` only decides whether `mde` is given. The inconclusive note now reads "an uncorrected t-test estimate puts the effect 80% power detects at this spread at about ±X (N tasks), before Holm and not for the sign-flip test". The `compare.py` module docstring, `compare_reach.min_detectable`, contract section 9, architecture.md and the changelog fragment say the same; the contract says why no Holm-calibrated figure is given (it would depend on the other rows' observed p, which a planning figure cannot know). How `mde` is computed and every JSON key and value are unchanged; the two tests that assert the note text take the new wording. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> --------- Signed-off-by: Anton Volkov <antonwolfy@gmail.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Co-authored-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Refs #94 (§5 stage F1a). Draft: D10 is still under review. No flag, default or output changes; the numbers are unchanged.
Summary
Scope is the registry and its checks only.
cli_flags.pyis read by no command; F1b moves each leaf onto it and tombstones old spellings.Review round (f309ca5 → this head)
docs/contributing-cli-flags.md(the 33rd Markdown file underdocs/, pastmax_docs: 32) is now a section ofCONTRIBUTING.md. The page, itsmkdocs.ymlnav entry and itsdocs/README.mdentry are gone.max_docsis unchanged:docs/**/*.mdcounts 32.intel/mainis 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) andnovelty 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 withmake cli-snapshot.--trialsis a banned synonym ofreps. 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.test_a_new_banned_synonym_fails_and_names_the_registry_spellingused--trials, which is now allowlisted, so it runs with that row removed.registry_problemsuntested per check) was fixed in f309ca5 and is pinned by one test per check.Friction for flag-adding PRs
make cli-snapshotand commit the JSON.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-kand--path-stabilityneed 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 forreps). Excerpt of the failing run (messages wrapped here, otherwise verbatim):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--trialsis now allowlisted; the probe now runs without that row. After this round:pytest tests/test_cli_flags.py tests/test_import_architecture.pygives37 passed.make ci-localon 393b7a5 (box run,TMPDIR=/tmp/...): every gate passes except two.pip-audit: CVEs againstfilelock==3.32.7, which main pins; this PR changes no dependency file, so it is inherited. The test step:2 failedon the first run (AF_UNIX path too longunder this box's long scratch path, and an unraisable socket warning), and both pass alone. The full suite on this head,pytest -n auto, gives9707 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
typecheckstep 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 thetypecheckstep 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. Thecli-interfaceworkflow (#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 noInternal. Thechangelog.d/README.mdroute for internal changes is theskip-changeloglabel; a maintainer may prefer that.🤖 Generated with Claude Code