Coverage records why a selected file failed, but not why a file was never selected #1345
NicolasRocchia
started this conversation in
Ideas
Replies: 1 comment 1 reply
|
I don’t really check the discussion module, so... does it still need to be resolved? |
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Context
I was studying how review tools represent a check that was planned and did not run, and read the run manifest and the session compare logic. The design is unusually careful:
terminal_statederived from the coverage sets andrun_failureonly, never from comment count or warnings;FailureClassa fixed set with a mandatory catch-all;ItemIDcontent-independent so a resume chain keeps a stable identity;rule_config_sha256andruntime_config_sha256pinned inexecution. Thenot_reviewedbucket inCompareResult, and its comment — "They are not resolved: nobody re-checked them" — is the sharpest statement of this problem I have found in a review tool.This is about a gap I could not close by reading, so I may have missed where it is handled.
The gap
Coveragehasselected,completed,reused,failedandwaived. A file that never became a candidate — removed byuser_exclude,default_path,unsupported_ext,provider_directory,too_large— is in none of them.model.ExcludeReasonis already a closed vocabulary of nine values, andPreviewcarrieswill_review,exclude_reasonandexcluded_count. ButExcludeReasonappears only undercmd/,internal/model,internal/agentandinternal/scan. Greppinginternal/session/forExcludeReasonorExcludedreturns nothing: the exclusion reason reaches the terminal output andocr delegate preview, and stops there. It is not in the manifest and not in the session JSONL.Two consequences follow:
A run where 2 of 40 changed files were excluded finalizes as
terminal_state: completewith 38 selected items. The denominator shrinks instead of the gap becoming visible. From the manifest alone, "there was nothing else to review" and "those two were deliberately not looked at" are the same record.executionpinsrule_config_sha256, so a consumer can prove which rules ran, but not which files those rules removed. Given how much of the rest of the manifest exists to make coverage auditable after the fact, that asymmetry reads as unintended rather than as a decision.Proposed change (small)
Add an
excluded []CoverageItemset toCoverage, populated at selection time, withclassificationcarrying the existingExcludeReasonvalue andreasonleft empty. Then either count a non-emptyexcludedin theterminal_statederivation, or state explicitly in the schema docs thatcompletemeans complete over the selected set.No new vocabulary is needed — the values already exist and are already closed. The change is where they are written down.
Alternatives I considered
Keeping it in the preview only, and asking users to save
ocr delegate previewnext to the run: it works, but it is a second artifact that has to be kept in sync by hand, and the preview is taken before the run rather than recorded by it. Reconstructing exclusions later fromrule_config_sha256plus the repo state: possible in principle, not reproducible once the rules or the working tree change.Why I think this earns a field
I maintain a corpus of 44 adversarial-review records under a schema that requires declaring what a review round could not close. 37 of its 46 residue items are checks that were planned and did not run. Once the not-run case is recordable at all, it is not a rare edge — it is most of what the record ends up containing. A manifest that can only describe selected work under-reports by construction, and the under-report is the part nobody can see.
I can send a PR if the direction is welcome, or open a formal Feature Request if you prefer that path.
—
Nicolás Rocchia · https://disensor.dev
All reactions