You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ci: report user-facing CLI surface changes on every PR - #88
Every change to how users call dokimi (commands, options, arguments, defaults, choices) is now flagged on the PR, mirroring abicheck's cli-interface-check. The new cli-interface workflow dumps the argparse CLI surface of the base tip and of the PR head, diffs them, and when they differ labels the PR cli-interface-change and keeps a sticky comment listing each added/removed/changed command and option. The label is removed once the surface matches base again. Reports only; never fails a PR.
Changes
dokimi/scripts/cli_surface.py: dump (walks dokimi.cli.build_parser(), incl. nested subparsers; stable JSON) and diff (exit 0 same / 1 differs, --markdown for the comment).
.github/workflows/cli-interface.yml: base tree via git archive (no git state change, so make ci-local runs it too); label + sticky comment via SHA-pinned actions/github-script; job-scoped pull-requests/issues: write.
dokimi/tests/test_cli_surface.py: 10 tests.
Risk
Low: new workflow, no package code touched. On fork PRs the token is read-only, so labeling/commenting is skipped with a warning and the diff is shown only in the job summary (no label or comment).
Testing
pytest tests/test_cli_surface.py: 10 passed.
make ci-local CI_LOCAL_ARGS='--offline': new step and static checks pass; only failure is the IPv6 test (no IPv6 in sandbox, unrelated).
Manual: a temporary demo subcommand produced + added command \dokimi demo``.
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.38%. Comparing base (61e0355) to head (e4ef0b0). ⚠️ Report is 5 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Add dokimi/scripts/cli_surface.py (dump the argparse tree, diff two dumps)
and the cli-interface workflow: it diffs the CLI surface of the base tip and
the PR head and, when they differ, labels the PR cli-interface-change and
keeps a sticky comment listing added, removed and changed commands/options.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Y9BUfAXTy2iFSheZYGGXN
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…mands
Address review: compare positional argument order, keep --version (not its
value), record whether a subcommand is required, list the params of added and
removed commands, drop issues: write, and only update the bot's own comment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Y9BUfAXTy2iFSheZYGGXN
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…uired status
The snapshot now serializes group membership (group_key) and group-level
required status (group_required) for each parameter. This allows diff_surfaces
to detect changes to which parameters are in mutually exclusive groups and
whether those groups are required. Adds regression tests that verify both
group membership and group-level required changes are detected.
Fixes copilot-pull-request-reviewer feedback on PR intel#88.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Confirmed detection gaps allow changes to accepted CLI invocations to go unreported.
No open issues 🎉
3 previously missed issues
In code that hasn't changed since last review
Detect restrictions introduced by empty choices
dokimi/scripts/cli_surface.py:64
An empty choices collection is not equivalent to unrestricted choices. Changing choices=None to choices=[] makes argparse reject every explicit value, but both serialize as None, so the diff misses the restriction. Check action.choices is not None and add a regression test.
Account for subparsers in positional argument ordering
dokimi/scripts/cli_surface.py:85
Subparsers consume a positional slot, but this branch does not advance position. Moving a parent argument across add_subparsers() therefore produces an empty diff even though the accepted invocation changes from dokimi TARGET run to dokimi run TARGET. Count the subparser in positional ordering and add a regression test for this move.
Detect changes to built-in help options
dokimi/scripts/cli_surface.py:88
Excluding _HelpAction hides removal or renaming of -h/--help. Setting add_help=False produces an empty diff even though dokimi --help changes from success to an unrecognized-argument error. Include help actions while continuing to omit help text. Add a regression test and update test_nested_subcommands_and_hidden_options_are_walked to select --hidden explicitly rather than assuming it is the only parameter.
The reason will be displayed to describe this comment to others. Learn more.
Review at a67e1e7. The workflow is well put together (SHA pins, job-scoped permissions, BASE_REF passed through env:, the label and sticky-comment logic). The problems are in what the diff can see. Three things are worth fixing before merge, all inline.
make ci-local: 20 passed, 0 failed, 4 skipped. This includes the new cli-interface.yml step and the changed-line coverage check.
Reproductions for the code findings, with no model or network access.
A mutation run on FIELDS: dropping seven of the twelve entries still passes all 19 tests in test_cli_surface.py.
I agree with Copilot's three points at cli_surface.py:64, :85 and :88. I ran the first: choices=None against choices=[] gives an empty diff.
Checked, no issues
Real CLI: dumping the base tree and the head tree and diffing them with the head script gives "No user-facing CLI surface changes detected." The dump has no object addresses.
Workflow shell: the rc handling is right (0 or 1 becomes changed, above 1 fails the step). PYTHONPATH selects each tree's own dokimi. zizmor reports nothing.
Markdown fence: a default holding three backticks and newlines cannot close the ```diff fence, because repr() escapes the newlines and every body line starts with a diff marker.
group_key was f'group_{index}' which changed when a new group was added
before an existing one, even though the group members stayed the same.
This caused reordering and new groups to falsely report unchanged options
as changed.
Fix: key the group by its sorted member keys instead. Update tests
to verify the new group_key format.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Preserve empty choices when serializing CLI restrictions
dokimi/scripts/cli_surface.py:94
choices=[] rejects every supplied value, while choices=None allows unrestricted values. This truthiness check serializes both as None, so introducing or removing that restriction produces no CLI-change report. Preserve empty collections with an explicit is not None check and add a regression test.
Advance positional index for subparser actions
dokimi/scripts/cli_surface.py:124
The subparser action does not advance position, so moving a parent positional across it is invisible. For example, placing workspace before versus after the run subparser changes the accepted invocation from dokimi demo run to dokimi run demo, but both snapshots record workspace at position 0 and produce an empty diff. Count the subparser as a positional slot and add a regression test.
…der wording
- Add `issues: write` permission to github-script step (uses github.rest.issues.* APIs)
- Serialize choices using _value() with repr() sorting, preserving type info (e.g., [1,2] vs ['1','2'])
- Add test for mixed-type choices; verify old code fails on it
- Change header from "options changed" to "parameters changed" (includes arguments)
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Verified that some changes to accepted CLI invocations produce no report.
No open issues 🎉
3 previously missed issues
In code that hasn't changed since last review
Distinguish unrestricted choices from empty choices
dokimi/scripts/cli_surface.py:94
This truthiness check serializes both choices=None and choices=[] as None. Argparse accepts unrestricted values for the former but rejects every supplied value for the latter, so adding or removing an empty choice restriction produces no CLI-change report. Check explicitly for None and add a regression test for this transition.
Detect changes to argparse allow_abbrev
dokimi/scripts/cli_surface.py:104
Changing allow_abbrev produces identical snapshots even though it changes accepted invocations. For example, --ver invokes --version with abbreviation enabled but exits with status 2 when disabled. Record parser.allow_abbrev for each command, compare it in diff_surfaces, and add a regression test so this change is reported.
Report changes to help option aliases
dokimi/scripts/cli_surface.py:127
Skipping _HelpAction hides changes to help options. Replacing the default -h/--help action with a --help-only action produces no diff, although dokimi -h changes from success to exit status 2. Include help actions in the snapshot; help prose is already excluded. Add alias-removal coverage and update the nested-parser test to select the hidden option rather than assume it is the only parameter.
The head is the merge commit made when the event fired; the base was the
branch tip at job start. A CLI change merged to main in between (queue,
re-run) was reported as removed by the PR. merge-base is HEAD^1 on CI and
the fork point under make ci-local.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Rewording the docstring or a comment of positive_int, shared by 69 options,
reported all 69 as changed. Hash the AST without docstrings instead; the new
test also pins that a changed literal in the body is still reported.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
choices=[] rejects every value and None accepts any, but the truthiness
check dumped both as None, so adding or removing the restriction was silent.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Moving a positional across add_subparsers() turns `dokimi WS run` into
`dokimi run WS`, but both dumped the positional at index 0.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Skipping _HelpAction hid add_help=False and a dropped -h alias. Help text
is not dumped, so only the option itself is compared.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
With it off, `--ver` stops meaning `--version` and exits 2; the dump had
no field for it, so the change gave an empty diff.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Fork PRs: a fork PR that changes the CLI gets no label and no comment, because its token is read-only (this PR's run shows Issues: read, PullRequests: read). Only the job summary shows the change, and the check stays green. Every open PR here comes from a fork, so the label and comment would normally appear. Fix: post them from a workflow_run follow-up (treat the artifact as untrusted and check the PR number against head_sha), or reword the header and body to say summary-only.
A validator that reads module state, e.g. JUDGE_ROLES.pop(...) behind --judge-panel, rejects more input but gives an empty diff. Fix: state that limit in the docstring and body.
Pushed:
5595076: compare against the merge base, not the base tip at job start.
f8e2e23: hash a validator's AST, so docstring or comment edits are not reported.
991b861, 7ae6696, 8595953, 5f997d3: close the four open gaps (choices=[], subparser position, -h/--help, allow_abbrev).
… compared
A validator that consults module state at parse time can reject more input
without any change in the parser dump, so the diff is empty for it. Say so in
the module docstring, as the review asked.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Review at da43829. The four gaps from the last round are closed (choices=[], the subcommand's positional slot, -h/--help, allow_abbrev), the merge-base change and the dump-error path are right, and each has a test that fails on the old code. What is left is at the edges of the validator hash and in the error path. Three things are worth fixing before merge and two are nits, all inline.
make ci-local: 20 passed, 0 failed, 4 skipped; 5,222 tests. The cli-interface.yml step prints "No user-facing CLI surface changes detected." Changed-line coverage has no lines to measure, because scripts/ is outside --cov=dokimi.
Reproductions for the code findings, with no model or network access.
A mutation run on cli_surface.py: 69 mutants, 13 of which pass the full test file. Every FIELDS drop, the subparser position += 1, is not None, allow_abbrev and the _HelpAction skip are killed. The new tests for the five latest fixes fail on the module from f863272.
As @xaleryb noted, a fork PR gets no label and no comment (read-only token); the header at cli-interface.yml:6-12 and the module docstring still say they always appear.
Checked, no issues
Real CLI: the base tree and the head tree dump to the same surface with the head script. The dump is byte-identical on a second run and has no 0x, /tmp or /home strings.
Workflow:BASE_REF goes through env:, permissions are job-scoped, fetch-depth: 0 is set, and zizmor reports nothing. changed=error is read before the boolean, so an error is not taken for "no change".
Tests: offline. The workflow-step test runs the YAML step unchanged on a real --no-ff merge commit, with GIT_* and PYTHON* stripped and GIT_CONFIG_GLOBAL=/dev/null.
Repo rules: SPDX headers are present, no generated artifacts are committed, and no changelog fragment is needed because no package path is touched.
… the error path
Address review of intel#88:
- A validator's keyword-only defaults and a self-referencing closure are now
covered: the hash includes __kwdefaults__, and a function reached again
through its own closure is named, not hashed again (no RecursionError).
- A class used as type= (an enum, or a class with methods) is hashed by its
attributes, so a new enum member or a changed method is reported.
- Rename the per-parameter group variables so mypy's str/None assignment
error goes away.
- Add a workflow test for the changed=error path (head dump raises).
- Describe the workflow's base as the merge base, not the base branch tip.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Review at a0be71f. The five threads from the last round are mostly closed: the keyword-only default, the self-referencing validator, enum members, the header wording and the mypy error are fixed, and each has a test that fails on the old code. The class hash added for the enum thread has gaps of its own, and the error-path test leaves part of the guard unpinned. Two things are worth fixing before merge, both inline.
make ci-local: 20 passed, 0 failed, 4 skipped. The cli-interface.yml step passes.
test_cli_surface.py: 48 passed, also under PYTHONHASHSEED 0, 1 and 424242. It is offline.
Reproductions for the code findings, with no model or network access.
A mutation run on the new _value branches and on the workflow step.
Still open from my earlier review:
dokimi/tests/test_cli_surface.py:410: test_workflow_reports_error_and_passes_when_the_head_cannot_be_dumped asserts changed=error and exit 0, which kills exit 0 to exit 1. It does not read diff.md. Replacing the "could not run" text with x, or dropping the file= line from the error branch, still passes (2 passed). In that case the github-script step reads an undefined file, falls into its catch, and posts no comment. Deleting the rc -gt 1 guard passes too, as you noted. Suggestion: assert the "could not run" text in diff.md and the file= line in the output.
Checked, no issues
Real CLI: the head and base trees dump to the same surface with the head script ("No user-facing CLI surface changes detected."). The dump is byte-identical under four hash seeds and has no 0x, /tmp or /home strings. Every type= in it is a plain function, so the new class branch does not touch it today.
mypy: the group_key assignment error is gone. ruff is clean on scripts/ and tests/.
Recursion and keyword-only defaults:kw(3) against kw(30) is reported, and a self-referencing validator is dumped. Dropping __kwdefaults__ or the function _seen check fails the new tests.
Workflow:zizmor reports nothing; the step still ends in exit 0 with changed=error when cli.py is gone.
Error path (the still-open point in your review): the test now asserts the "could not run" text in diff.md and the file= output line. As you noted, the rc -gt 1 guard is still not reached by a dump failure, so that test does not cover it.
Inherited, static, class and property members: fixed in 9c28130 (see the inline thread).
The _seen guard and _CLASS_NOISE tests from the other open thread: added in 9c28130 (a self-referencing class, and a docstring-only change). The enum branch in that thread is not changed in this push and is still open on my side.
…in sorted order
A class used as type= is now hashed over its whole MRO, with static
methods, classmethods and properties hashed by their functions, and the
member order no longer changes the hash. The Limit paragraph names what
is still compared by name only. Tests cover each case, and the error
path of the workflow step now checks diff.md and the file= output.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Re the points in this comment, on top of the branch as it is now (9c28130):
Fork PRs get no label or comment (read-only token): this needs a choice between a workflow_run follow-up poster and summary-only wording. That choice is pending, so nothing is changed here.
Validators that read module state: the limit is stated in the module docstring (scripts/cli_surface.py, Limit: paragraph, since da43829). The PR description is not edited.
Fork PR tokens are read-only, so the label and comment are skipped there.
refactor(cli-surface): drop redundant enum branch in class hash
vars(cls) already carries _member_map_, so the extra member block added nothing.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Re @xaleryb's fork-PR point (#88 (comment)): option B chosen. Fork PRs have a read-only token, so the header of .github/workflows/cli-interface.yml and the PR body now say they see the CLI diff only in the job summary (no label or comment). A workflow_run poster can follow as its own PR if the label is wanted on forks. Pushed in cf28a0c.
Ran at cf28a0c: full suite, ruff, mypy, bandit. Also re-ran every earlier repro from this thread on Python 3.11-3.14, plus a base/head dump of the real CLI under several hash seeds. All pass. The only failing CI check is the License header check, and it fails the same way on main.
Open: the scripts/cli_surface.py module docstring (lines 7-11) still says the workflow labels the PR and posts a sticky comment, so each change "stays on record in the PR". On a fork PR (read-only token, as in this PR's own run) neither happens, and the diff is only in the job summary. Expected: the same fork sentence that the workflow header got in cf28a0c.
The reason will be displayed to describe this comment to others. Learn more.
Review at cf28a0c. The three threads from the last round are closed: the error-path test now pins diff.md and file=, the class hash walks the MRO and handles static/class methods and properties, and the enum block is gone. The new class branch still has gaps of its own. Three things are worth fixing before merge and one is a nit, all inline.
make ci-local: 25 passed, 2 failed, 4 skipped; 6,407 tests. The cli-interface.yml step passes. Both failures are the gitleaks job, which could not download its binary; gitleaks run by hand over the PR's 19 commits finds no leaks. Changed-line coverage has no lines to measure, because scripts/ is outside --cov=dokimi.
test_cli_surface.py: 55 passed. A 92-mutant run on cli_surface.py: 37 survive the file, about 11 of them equivalent.
Reproductions for the code findings, with no model or network access.
I agree with @xaleryb that the cli_surface.py module docstring (lines 7-11) still says the PR is labelled and commented with every change "on record", while a fork PR gets only the job summary.
Still open from my earlier review: none; the three threads are resolved.
Checked, no issues
Real CLI:type= in the real parser is plain functions, so the class branch does not touch it today.
Workflow:zizmor reports nothing; BASE_REF goes through env:, permissions are job-scoped, and the fork note in the header matches the steps (diff.md goes to the job summary without a write token).
Repo rules: SPDX headers are present, no generated artifacts are committed, no changelog fragment is needed (no package path is touched), and the tests are offline.
Enum types: adding a member, changing a value, renaming a member and adding an alias are all still reported after the enum block was removed.
The reason will be displayed to describe this comment to others. Learn more.
Builtin bases and functools wrappers are invisible to the class hash, so a real change to a type= class gives an empty diff.
Builtin classes are skipped both in the attribute walk (:120) and in bases (:124), so changing class C(int) to class C(str) changes which values type=C accepts and not the digest. lru_cache and cached_property members fall to the name-only branch (:130-132), so their body is not hashed either.
class CInt(int): pass
class CStr(str): pass # renamed to CInt in the other tree
cs._value(CInt) -> modv.CInt#d5a51e9397b3
cs._value(CStr) -> modv.CInt#d5a51e9397b3
class CCached:
@functools.lru_cache
def run(self): return 1
cs._value(CCached.__dict__['run']) -> 'run' # return 2 gives the same
The Limit paragraph (:17-21) lists callable instances, functools.partial and metaclasses only. Nothing in the real CLI uses a class as type=, so this is a limit, not a live miss.
Suggestion: keep builtin bases in bases (drop only object), and either hash __wrapped__/func for lru_cache/cached_property or add them to the Limit paragraph.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in dccdf2a: bases now keep builtin bases (only object is dropped), so int -> str changes the digest (test_a_builtin_base_of_a_class_type_is_a_change). lru_cache/cached_property bodies are not hashed; this is now stated in the Limit: paragraph of the module docstring.
The reason will be displayed to describe this comment to others. Learn more.
An enum whose member value has no stable repr gives a different digest on every run.
With the dedicated enum block gone, an enum class reaches the class branch through vars(cls)['_member_map_'], and its members are turned into text by repr(value) here, before the address-stripping fallback at :133-134 can run.
class Obj(enum.Enum):
a = object()
cs._value(Obj) # run 1: modv.Obj#e5cf11b5edbf
cs._value(Obj) # run 2: modv.Obj#45e9483cf2d1
The base and head dumps are made in separate processes, so an unchanged type=Obj is reported as a change and the PR gets the label. Enums of ints and strings are fine, so this is rare.
Suggestion: for an enum, return [repr(type(value)), _value(value.name), _value(value.value, _seen)] instead of repr(value).
The reason will be displayed to describe this comment to others. Learn more.
Fixed in dccdf2a: enum members dump as [repr(type(value)), name, value]. I also excluded _value2member_map_ from the class walk: it is keyed by str(value), which carried the same address. The test loads an Enum with an object() value twice and expects an empty diff (fails on old code).
The reason will be displayed to describe this comment to others. Learn more.
Four parts of the new class branch pass the full test file when broken.
Mutation check on the 55-test file, run one mutant at a time (each leaves 55 passed):
attrs.setdefault(k, v) to attrs[k] = v (:123): the base class's definition wins over the subclass's override. A loosened override gives an empty diff.
+ repr(bases) removed (:125): swapping class Check(A) for class Check(B), where A and B have the same members, is no longer seen.
(value.fget, value.fset, value.fdel) cut to (value.fget,) (:101): test_a_class_type_is_compared_by_its_static_method_classmethod_and_property only changes the getter.
sorted(...) replaced by a plain list over a set (:95): a multi-element string set then dumps in hash-seed order, so base and head dumps can differ with no change.
Suggestion: add four cases to test_cli_surface.py: a subclass that overrides a base method (loosen only the override); two classes with identical members and different bases; a setter-only change in the property template; and a three-string frozenset default dumped under two PYTHONHASHSEED values.
The reason will be displayed to describe this comment to others. Learn more.
Added in dccdf2a: subclass override wins over base; same members / different bases; property setter-only change; frozenset default under 4 PYTHONHASHSEED values. These kill the mutants but pass on the unmutated old code by design.
The reason will be displayed to describe this comment to others. Learn more.
Nit: a crashed diff is reported as "CLI changed", and the rc -gt 1 guard cannot fire.
An uncaught Python exception exits with status 1, the same as "surfaces differ". With a truncated base dump, cli_surface.py diff bad.json bad.json --markdown prints a JSONDecodeError traceback and writes nothing to stdout. The step then writes changed=1 and an empty diff.md, so the next step adds the label and posts a comment with only the marker. No test reaches the guard either: deleting :79 still gives 55 passed.
Both dumps come from the same script, so this needs a bug in diff_surfaces to happen. Suggestion: in main() catch (OSError, ValueError, KeyError) in the diff branch, print the error to stderr and return 2, and send rc above 1 to the changed=error branch instead of exit "$rc".
The reason will be displayed to describe this comment to others. Learn more.
Fixed in dccdf2a: main() catches OSError/ValueError/KeyError in diff, prints to stderr and returns 2; the workflow sends rc>1 to the same changed=error branch (report_error, exit 0) instead of exit $rc. Tests: main exits 2 on a truncated dump; the workflow step with a truncated head.json gives changed=error.
Re @xaleryb's open point (docstring of scripts/cli_surface.py): fixed in dccdf2a. The module docstring now says a PR from a fork has a read-only token and gets the diff in the job summary only, with no label or comment (same sentence as the workflow header). Antonwolfy's four inline points from cf28a0c are answered on their threads at the same sha.
Re-checked at e4ef0b0: full suite, ruff, mypy and bandit on main merged with this branch, plus every earlier repro from this thread (choices=[], the subcommand's positional slot, -h/--help, allow_abbrev, the merge base, validator docstring edits, the fork wording in the docstring, enum object() values on 3.11-3.14 under two hash seeds, int -> str base, truncated dump -> exit 2 / changed=error). All pass; nothing open, nothing pushed.
…r raise
Main grew four baselined or near-cap items and added three functions over
complexity 10 before the gate landed: test_repo_environment.py 1369 -> 1371
(intel#147), manifest._check_task 21 -> 22 (intel#113), ci_local.run 28 -> 29 (intel#146),
cmd_repo_validate 10 -> 11 (intel#150), cli_surface._value 17 (intel#88),
research_register.check 19 and main 15 (intel#139). Each entry names its PR.
--update after the seed lowers nothing. Now 15 files and 98 functions.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* ci: no-growth ratchet for file length and function complexity
A first-party .py file over 800 lines (1000 under tests/) or a function
over cyclomatic complexity 10 (ruff's C901 count, computed with ast so the
test job needs no ruff) now fails tests/test_growth_ratchet.py unless
scripts/growth_baseline.json records it. A recorded value is exact: the
code may shrink but not grow, and a shrunk, gone or back-under-cap entry
must be lowered or removed (--update does that, never raising). An entry
raised or added relative to the base branch needs a rationale of at most
300 characters.
The baseline holds the 14 files and 81 functions over the caps today.
`check_growth.py --report --markdown` adds the totals to the static job's
step summary, non-blocking.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix(ci_local): pass resolved base ref to growth check via DOKIMI_GROWTH_BASE
ci_local.py resolves the base ref to handle non-origin remotes, but did not pass
it to the pytest environment. This caused the growth check to use the wrong
baseline or skip the rationale check if the baseline was missing, allowing
misleading local CI passes when using --remote with a non-origin remote.
Pass the runner's resolved base_ref as DOKIMI_GROWTH_BASE environment variable
to the subprocess steps. Add a regression test that verifies the check correctly
uses a non-origin remote's baseline.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* ci(growth): baseline evaluate_novelty at 26, as #72 left it on main
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* feat(ci-local): pass resolved base_ref as DOKIMI_GROWTH_BASE env var
ci_local.py now passes the resolved base reference to workflow steps as
DOKIMI_GROWTH_BASE, allowing check_growth.py to use it without requiring
--base argument when ci_local is used to run workflows locally.
Adds test to verify the env var is passed to all subprocess steps.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix: restore the files f105136 reverted by mistake
f105136 was built on a stale tree and deleted the #72 and #84 changes
from this branch. Bring every file back to 6c4cc20; only the
test_ci_local.py addition of f105136 stays.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix(growth): a raised entry needs a new rationale; test loop/try else and match guards
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix(ci_local, check_growth): propagate DOKIMI_GROWTH_BASE to isolated jobs and guard TryStar
Two fixes for growth ratchet compatibility:
1. Isolated jobs (package-smoke) did not receive DOKIMI_GROWTH_BASE from base_env,
causing the growth check to compare against the wrong baseline. Now included in env.
2. ast.TryStar is only available on Python 3.11+ (introduced with except*). Guarded
with hasattr() so the script remains compatible with Python 3.10.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix(ci_local, check_growth): cover isolated-job env and drop dead TryStar guard
Add a regression test for DOKIMI_GROWTH_BASE reaching the isolated
package-smoke job, and a try-star complexity case. ast.TryStar always
exists on the supported Python (>=3.11), so the hasattr guard is removed.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* refactor: split functions over the complexity cap
Extract helpers from cmd_answers_generate (runtime config, question loading,
output path) and from the import-architecture test (import targets, owner
module, Tarjan component pop, spawn-import aliases) so each is under the cap.
cmd_answers_generate is back under its cap, so its baseline entry is removed.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* test(answers): cover the helpers answers generate was split into
The refactor moved config loading, question loading and output-path choice
into helpers that the existing tests never reach, leaving 12 changed lines
uncovered (diff-cover gate: 61%, needs 80%). The new tests cover the
missing-, empty-, invalid- and non-mapping config cases, the question load
error, the output-path precedence, and the command end to end over fake
Answerer and documentation-source clients.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* ci(growth): take the #101 growth into the baseline; changelog fragment
Main's #101 grew dokimi/novelty/bundle.py to 1016 lines and
evaluate_novelty to complexity 29 after the baseline was cut, so the
ratchet failed on code this PR does not touch. Raise both entries with a
rationale, as the #72 entry already does.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix(growth): compare rationale against the fork point, not the base tip
The rationale check read the baseline at the tip of origin/<base>. Once main
lowers an entry, every branch behind it, and a re-run CI merge commit, was
told it raised that entry. Read it at git merge-base HEAD <ref> instead.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
* test(growth): pin the C901 count and guards that no test reached
Eleven single-line mutants of check_growth.py passed the whole suite: defs in
except/case bodies, async for, nested async defs, class bodies, `(_ as z)`,
duplicate names, GITHUB_BASE_REF, cli.py, dot dirs, non-int entries and the
rationale strip. Expected counts come from ruff C901 at max-complexity 0.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
* test(growth,answers): pin the match-case counts, the subdirectory baseline read, and the top_k default
Three review gaps with no failing test: a decision inside a case body and a refutable
as-pattern last case in the complexity count; load_base_baseline when the project sits
in a subdirectory of the git root (the ./ prefix); and the answers command's top_k
default from retrieval.top_k when the flag is unset.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* test(answers): make the flag row differ from config and default; fix growth comments
The flag row used top_k=3, which is also the built-in default, so a
command ignoring --top-k still passed (mutant: if True:). Use 5 with a
config of 7. Correct the test and function counts in check_growth.py
comments and drop the unused _product_config assignment in answers.py.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* ci(growth): --seed mode, reseed baseline after merging main, document --update after rebase
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* fix(growth): the --seed rationale does not explain a raise against a baselined base
After this lands, `--seed` on a branch that grew llm.py wrote the entry at
the new size with "pre-existing when the gate was introduced", and the gate
passed. The rationale check now treats that text as missing, so only the
one-time seeding (base with no baseline) can use it.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
* ci(growth): reseed the baseline after the merge of main (main has no baseline)
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* ci(growth): seed the baseline after merging main, with a rationale per raise
Main grew four baselined or near-cap items and added three functions over
complexity 10 before the gate landed: test_repo_environment.py 1369 -> 1371
(#147), manifest._check_task 21 -> 22 (#113), ci_local.run 28 -> 29 (#146),
cmd_repo_validate 10 -> 11 (#150), cli_surface._value 17 (#88),
research_register.check 19 and main 15 (#139). Each entry names its PR.
--update after the seed lowers nothing. Now 15 files and 98 functions.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
* ci(growth): re-seed the baseline after merging main again (#73, #115)
#73 grew test_judge_validity.py 1423 -> 2235, test_judge_reliability.py
1505 -> 1695, model_compare.py 1296 -> 1297, cmd_eval_judge_validity
25 -> 29, and took degrade.build_pairs (17) and
judge_sequential.run_sequential (12) over the cap. #115 grew
l0 _build_names 14 -> 15 and shrank _cmake (now under the cap, entry
dropped) and Scan.resolve_path (20 -> 19). Each raise names its PR.
Now 15 files and 99 functions.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
---------
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Co-authored-by: Alexander Rybkin <alexander.rybkin@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
antonwolfy
left a comment
Summary
Every change to how users call dokimi (commands, options, arguments, defaults, choices) is now flagged on the PR, mirroring abicheck's
cli-interface-check. The newcli-interfaceworkflow dumps the argparse CLI surface of the base tip and of the PR head, diffs them, and when they differ labels the PRcli-interface-changeand keeps a sticky comment listing each added/removed/changed command and option. The label is removed once the surface matches base again. Reports only; never fails a PR.Changes
dokimi/scripts/cli_surface.py:dump(walksdokimi.cli.build_parser(), incl. nested subparsers; stable JSON) anddiff(exit 0 same / 1 differs,--markdownfor the comment)..github/workflows/cli-interface.yml: base tree viagit archive(no git state change, somake ci-localruns it too); label + sticky comment via SHA-pinnedactions/github-script; job-scopedpull-requests/issues: write.dokimi/tests/test_cli_surface.py: 10 tests.Risk
Low: new workflow, no package code touched. On fork PRs the token is read-only, so labeling/commenting is skipped with a warning and the diff is shown only in the job summary (no label or comment).
Testing
pytest tests/test_cli_surface.py: 10 passed.make ci-local CI_LOCAL_ARGS='--offline': new step and static checks pass; only failure is the IPv6 test (no IPv6 in sandbox, unrelated).demosubcommand produced+ added command \dokimi demo``.Checklist
pytestpassesruff,mypy, and schema checks passGenerated by Claude Code