Repository navigation
fix(repo): scan says why it is incomplete - #295
Conversation
`repo scan` marked a scan `incomplete` (a missing `also_scan` file, no guidance file, an unreadable guidance file or Bazel build input) but its text output never said so: a missing `also_scan` file printed exactly like a harmless glob that matches nothing, and `--fail-on-fail` exited 1 under `fail 0` with no message. scan_repo now keeps the warnings that make the scan incomplete as their own list, `incomplete_reasons` (the same strings as in `warnings`, in the same order; `[]` when complete), decided in `_coverage_warnings` and next to the unread-file warnings; `incomplete` is now `bool(incomplete_reasons)`. `repo_readiness.v1` gains it as an optional property, a widening, so the version stays v1 and reports already written stay valid. The text output prints `incomplete: <reason>; ... (repo scan --fail-on-fail exits 1)` after the warnings, and nothing new for a complete scan. When `--fail-on-fail` exits 1 on an incomplete scan it prints `✗ scan incomplete: <reasons>` on stderr; the exit code is unchanged. The guidance-only manifest in docs/reviewing.md listed `also_scan: ["CONTRIBUTING.md"]`, so copied into a repository without that file every scan was incomplete and the gate could never pass. The example drops it and the text says what `also_scan` is for; contract §8 documents the new line, the stderr message and `incomplete_reasons`. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
|
@exai-bot /review |
exai-bot
left a comment
There was a problem hiding this comment.
Note
AI-generated review of 4fa7783984bf, requested by @antonwolfy, using the default review skill. It can be wrong: verify a finding before acting on it. React with a thumbs up or down on a finding to tell the bot whether it helped.
CI on this commit, at review start and at posting: 15 passed, 4 running, 2 skipped.
Model us.anthropic.claude-sonnet-5-5, effort default, 5 of 155 turns, 0.3 of 120 minutes, 124k in / 2k out tokens.
The change adds incomplete_reasons to the scan payload and schema. The text output now prints an incomplete: ... line, and --fail-on-fail prints ✗ scan incomplete: ... on stderr when it exits 1 on an incomplete scan. The reasons are built from the same strings as the matching warnings entries, in the same order, and incomplete is bool(reasons), so it has the same causes as before. Exit codes are unchanged, the schema change only widens v1, and the new tests cover the stderr line, the text line, the JSON field and the glob-only case. I found no defects.
No issues found.
Repository checks
Each ran as the head of this pull request defines it.
| Check | From | Result | Note |
|---|---|---|---|
python -m pytest tests/test_repo_l0.py tests/test_repo_l0_cases.py tests/test_repo_l0_parity.py tests/test_docs_code_refs.py -q -x |
AGENTS.md | not run | pytest is not installed in the container and there is no network to install it |
ruff check dokimi/commands/repo.py dokimi/repos/l0/__init__.py tests/test_repo_l0.py |
.pre-commit-config.yaml | not run | ruff is not installed in the container |
Not verified
I did not run the tests or linters, because the container has no pytest or ruff and no network. I relied on reading the diff. The PR description reports make ci-local and the targeted tests as passing.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #295 +/- ##
==========================================
+ Coverage 93.28% 93.50% +0.22%
==========================================
Files 272 273 +1
Lines 35119 35576 +457
Branches 5964 6042 +78
==========================================
+ Hits 32761 33267 +506
+ Misses 1544 1492 -52
- Partials 814 817 +3
🚀 New features to boost your workflow:
|
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 9 selected item(s). Review details
|
Review comment 4415624342: the parity test of unreadable build inputs derived its expected incomplete_reasons from the warnings under test, so a payload that copied every warning into incomplete_reasons still passed. The scan now also has a guidance glob that matches no file; the test asserts its warning is in `warnings` and that incomplete_reasons is exactly the two literal `build file not scanned: ...` entries, in order. The `_scan` helper takes the guidance globs for this. Review comment 4415624349: the scan without --fail-on-fail now asserts stderr is empty, so the `✗ scan incomplete: ...` line is pinned to the gate and not printed by a plain scan. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
|
Review round in 64fa41a, tests only. Both OpenCodeReview points held: a mutant survived each of them at 4fa7783, and both are caught now.
The reordering half of the first point was already caught; the reply in that thread says how. |
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 64fa41a. The change is right: incomplete now comes from the same list as incomplete_reasons, so it has exactly the causes it had before, and the reasons are the matching warnings strings in their order. One small gap with an invalid --static is in the line comment.
What I ran, at 64fa41a:
make ci-local --offline: 17 checks passed, 0 failed, 13 skipped (network-only); tests 10,135 passed, 57 skipped.repo scanof a tree withoutCONTRIBUTING.mdinalso_scan, with an invalid--staticand--fail-on-fail --json: exit 1, stderr has only the--staticerrors;incomplete_reasonsin the JSON is right.
Checked, no issues
- Same gate as before: the warnings keep the old order (globs,
also_scan, no guidance, unreadable guidance, unreadable build inputs), and only the unmatched-glob warnings stay out ofincomplete_reasons. - Schema:
incomplete_reasonsis optional inrepo_readiness.v1, so older reports still validate;schema_compatpasses. - Tests: the new assertions read
incomplete_reasonsand the stderr line, which do not exist on the base, so they fail there; the two OpenCodeReview points are fixed in 64fa41a as the replies say. - Docs and changelog: the contract doc,
reviewing.mdand theFixedfragment match the code; droppingalso_scan: ["CONTRIBUTING.md"]from the guidance-only example removes a manifest that made scans of repos without that file incomplete.
…s invalid An invalid --static document exited 1 before the gate ran, so an incomplete scan printed the --static errors but not the "✗ scan incomplete: ..." line that the contract promises. Run the gate first; both paths still exit 1. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 9 selected item(s). Review details
|
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the changes that explain why repository scans are incomplete.
No open issues 🎉
What changed in this PR
0 new findings; nothing to fix.
This PR makes incomplete repository scans explain what was missed, distinguishing gate-failing conditions from harmless warnings without changing exit codes.
Changes:
- Records ordered
incomplete_reasonsin scan results and adds the optional field to the existing report schema. - Prints incomplete reasons in text output and on stderr when
--fail-on-failrejects an incomplete scan. - Updates guidance examples, documentation, and regression tests.
| File | Description |
|---|---|
dokimi/tests/test_repo_l0.py |
Tests scan reasons, output, artifact validation, and gate behavior. |
dokimi/tests/test_repo_l0_parity.py |
Checks ordered build-input reasons exclude harmless glob warnings. |
dokimi/tests/test_repo_l0_cases.py |
Checks unreadable guidance appears in incomplete reasons. |
dokimi/dokimi/schemas/repo_readiness.v1.json |
Adds optional incomplete_reasons while preserving older report validity. |
dokimi/dokimi/repos/l0/__init__.py |
Collects incomplete reasons and derives scan completeness from them. |
dokimi/dokimi/commands/repo.py |
Displays reasons and extracts gate handling into _scan_gate. |
dokimi/docs/reviewing.md |
Removes the example’s required extra file and explains also_scan. |
dokimi/docs/repo-evaluation-contract.md |
Documents the new field and diagnostic output. |
dokimi/changelog.d/20261029_150000_antonwolfy_scan-incomplete-reasons.md |
Records the scan-diagnostic fix and example correction. |
Review effort: Balanced
|
PR head at check: 64fa41a (unchanged on 10-29). Pushed sha: 83e3afc. Anton, the open thread on
Tests: Questions left: none. The other two threads in this PR ( |
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
LGTM
I checked 83e3afc. The --static gap from my first round is fixed: _scan_gate now runs before the --static exit, so an incomplete scan with an invalid --static names its reason on stderr, and both paths still exit 1. Everything from the first round still holds.
What I ran, at 83e3afc:
make ci-local --offline: 17 checks passed, 0 failed, 13 skipped (network-only); tests 10,136 passed, 57 skipped.
Checked, no issues
- Gate order: a failed claim with an invalid
--staticexits 1 from_scan_gate, as it did from the--staticexit before, so no exit code changes. - Test:
test_an_invalid_static_file_still_says_why_an_incomplete_scan_failschecks both stderr lines; on 64fa41a thescan incompleteline is missing, so it fails there.
Summary
Found while evaluating IntelPython/dpnp#3099 (AGENTS.md guidance) with dokimi:
repo scanmarks a scan
incompletebut its text output never says so, and--fail-on-failthen exits 1with no message. dpnp at
d8fb283, with a guidance-only manifest modelled on the one indocs/reviewing.md(
globs: ["**/AGENTS.md", "**/CLAUDE.md", ".github/copilot-instructions.md", ".github/instructions/*.md"],also_scan: ["CONTRIBUTING.md"]; dpnp has no CONTRIBUTING.md), gives onmain:and
"incomplete": truein the JSON. The one warning that makes the scan incomplete, themissing
also_scanfile, prints exactly like the three harmless glob warnings, and theperson reading it took the glob warnings for the cause. With no failing claim,
--fail-on-failprints
fail 0and exits 1 with nothing on stderr. The example manifest indocs/reviewing.mdsets this up: copied into a repository without a CONTRIBUTING.md, every scanis
incompleteand the gate can never pass. The rule itself is right and documented (contract§8: "a file the gate must see belongs in
also_scan"); its visibility and the example were not.The same scan now prints, after the warnings:
and
--fail-on-failexits 1 with✗ scan incomplete: guidance/also_scan matches no file: CONTRIBUTING.mdon stderr.
Changes
repos/l0/__init__.py:_coverage_warningsreturns the warnings that make the scanincomplete (a missing
also_scanfile, no guidance file) next to all warnings, andscan_repoadds the unreadable guidance files and Bazel build inputs to them. The payloadgains
incomplete_reasons: the same strings as theirwarningsentries, in the same order,[]for a complete scan.incompleteis nowbool(incomplete_reasons), the same causes asbefore;
warningsis unchanged.schemas/repo_readiness.v1.json:incomplete_reasonsas an optional array of strings. Awidening, so the version stays v1 and is not in
required: reports already written stay valid.commands/repo.py(cmd_repo_scan): the text output printsincomplete: <reason>; <reason> (repo scan --fail-on-fail exits 1)after the warnings when thescan is incomplete, and nothing new otherwise. The gate moved into
_scan_gate(which keepscmd_repo_scanat its complexity baseline): when it exits 1 on an incomplete scan it prints✗ scan incomplete: <reasons>on stderr, in--jsonmode too. A failed claim on a completescan exits 1 as before, with nothing on stderr: its FAIL lines say why. Exit codes unchanged.
docs/reviewing.md: the guidance-only example no longer listsalso_scan; a paragraph after itsays what
also_scanis for and that a missing file makes the scanincompleteandrepo scan --fail-on-failexit 1.docs/repo-evaluation-contract.md§8:incomplete_reasons, theincomplete: ...line and thegate's
✗ scan incomplete: ...on stderr.tests/test_repo_l0.py): a missingalso_scanfile is named inincomplete_reasons, inthe text output's
incomplete:line after the warnings, and on stderr when--fail-on-failexits 1; a glob that matches nothing alone gives no
incomplete:line,[]and exit 0 under--fail-on-fail; both go through--out, so the report validates against the schema. Theno-guidance case of
test_fail_on_fail_follows_the_result_not_the_flagnow checks the stderrreason, and
test_scan_command_writes_a_valid_artifactthat a gate failing only on claimsprints nothing on stderr.
test_repo_l0_cases.pyandtest_repo_l0_parity.pycheckincomplete_reasonsfor an unreadable guidance file and unreadable Bazel inputs.Risk
The text output of an incomplete scan gains one line, and
--fail-on-failon an incompletescan writes one line to stderr. A complete scan prints what it did before. The JSON and the
--outreport gainincomplete_reasons; a consumer that rejects unknown keys ofrepo_readiness.v1would see it, and reports written before still validate. Exit codes areunchanged.
Testing
On a Linux review host, at
4fa7783(onmaine966422):make ci-local: 27 passed, 1 failed, 7 skipped (external-catalog-smoke: needs Docker andnetwork;
test-latest,docs.yml:deploy,pip-audit.yml:report: job condition false on a PR;cli-surface "Package the result": unknown expression; conda-forge-deps: no micromamba;
semgrep secrets: not installed, runs on the PR). 10188 tests passed, 4 skipped; changed-line
coverage 100%. The failure is
pip-audit: 10 advisories againstfilelock 3.32.7(fix4.0.9 / 4.2.1), which
main'srequirements.lock.txtandrequirements-ci.lock.txtpin.This PR changes no dependency or lock file; ci(deps): document and ignore ten more filelock 3.32.7 advisories #288 documents and ignores those advisories.
tests/test_repo_l0.py,test_repo_l0_cases.py,test_repo_l0_parity.py, withtest_docs_code_refs.py,test_growth_ratchet.py,test_audit_test_quality.pyandtest_import_architecture.py: 569 passed;ruff checkon the changed files andmypyon thepackage: clean.
The new and changed tests on
main's code: 6 fail (the two new tests, the no-guidance case oftest_fail_on_fail_follows_the_result_not_the_flag, both cases oftest_dangling_guidance_symlink_reported,test_unreadable_build_inputs_warn_and_mark_the_scan_incomplete).repo scanof this repository with a manifest whosealso_scannames a missing file:claims: 80 (ok 69, unresolved 11, fail 0), theincomplete: guidance/also_scan matches no file: ...line,✗ scan incomplete: ...on stderr, exit 1.Mutants, each run against the three scan test files; every one fails a test:
incomplete:line printed for a complete scan insteadtest_a_glob_that_matches_nothing_alone_keeps_the_scan_complete,test_a_missing_also_scan_file_is_named_as_the_causeincomplete:line droppedtest_a_missing_also_scan_file_is_named_as_the_causetest_a_missing_also_scan_file_is_named_as_the_cause,test_fail_on_fail_follows_the_result_not_the_flag[no guidance]test_scan_command_writes_a_valid_artifactincompletetest_a_missing_also_scan_file_is_named_as_the_cause,test_fail_on_fail_…[no guidance],test_cli_exit_codes_coverage_warningsreturns no reasonstest_a_missing_also_scan_file_is_named_as_the_cause,test_fail_on_fail_…[no guidance],test_cli_exit_codestest_a_glob_that_matches_nothing_alone_keeps_the_scan_complete,test_a_missing_also_scan_file_is_named_as_the_cause,test_fail_on_fail_…[no guidance]test_repo_l0_cases.pyandtest_repo_l0_parity.pyMerged together on
mainf0f95a2with the other PRs from this evaluation (fix(repo): scan says why it is incomplete #295 at4fa7783, feat(repo): compare records the start context, filters tasks, and flags inconclusive rows #296 atcad52dd, feat(repo): replay grades record each credited hit; repo report --hits lists them #297 at0a7539a, fix(novelty): a capped run says how far the cap reached #299 atfa453d8), on a workstation: the full suite has 10482 passed and 57 skipped, and the same 5 failures and 1 collection error asmainalone there (10456 passed). All six come from that machine's setup: nodocker, nohypothesis, and two tests that depend on its git and shell settings.Review round at
64fa41a(onmaine966422), tests only.test_unreadable_build_inputs_warn_and_mark_the_scan_incomplete(tests/test_repo_l0_parity.py) now adds a guidance glob that matches no file. It asserts that the glob's warning is inwarningsand thatincomplete_reasonsis the two literalbuild file not scanned: …entries, in order.test_a_missing_also_scan_file_is_named_as_the_cause(tests/test_repo_l0.py) now asserts that stderr is empty for a scan without--fail-on-fail. Mutants, all three caught by the new tests:incomplete_reasonsset to a copy of every warning: survived at4fa7783.4fa7783.✗ scan incomplete: …printed to stderr without the flag: survived at4fa7783.make ci-local: 24 passed, 4 failed, 7 skipped. 10187 tests passed, 4 skipped, 1 failed; changed-line coverage 100%. None of the four failures comes from this change:Scan commitsstep after it had no binary (exit 127).filelock 3.32.7advisories thatmain's lock files pin.test_reference_checks.py::test_the_regex_limit_is_shared_by_every_pattern_of_one_checktook 1.70 s against a 1.5 s limit. It passed on a rerun of its file (62 passed), and this branch does not touch that test or its code.Not run: the dpnp scan itself at this head (the output above is from the evaluation at
main).Revert
A plain revert. Reports written meanwhile keep an
incomplete_reasonskey that the revertedrepo_readiness.v1schema (closed,additionalProperties: false) would reject on load; drop thekey or rewrite them.
Checklist
pytestpassesruff,mypy, and schema checks passAGENTS.md.github/.licenserc.yaml)