fix(ci-formats): a check that could not complete is not a check that passed

[ChelseaKR]

Draft: this bumps report_schema_version to 1.2, which is a published contract. The last section names the two calls that are yours.

What was wrong

RuleEngine.run catches a check that throws, publishes one warning finding in place of everything the rule did not conclude — "This check could not complete. No package conclusion was made." — and sets the run's exit code to 2.

Console, HTML and the checklist print that sentence, so a person reads it. The two CI formats added in #104 count elements rather than reading them, and both counted this one as a rule that ran and was content.

Measured on main (6d948f7), driving the real check_package path over a real synthetic NOE package with one registry entry raising:

CLI exit code: 2
report.not_run entries: 6
summarize_counts: {'failure': 0, 'warning': 2, 'pass': 11, 'manual': 1, 'not_run': 6}

SARIF executionSuccessful: True
SARIF toolExecutionNotifications: 6

<testsuites name="ceqa-preflight" tests="20" failures="0" errors="0" skipped="6" ...>
<testcase name="PDF-006: Active PDF content: internal rule error" classname="ceqa-preflight.NOE">
  <system-out>WARNING PDF-006: This check could not complete. No package conclusion was made.</system-out>
</testcase>

A <testcase> whose only child is <system-out> is a passing test case. A JUnit job summary totals that run as 20 tests, 0 failures, 0 errors, 6 skipped, 14 passed — with the check that crashed among the fourteen — on a run the tool itself exits 2 for. And errors="0" was a hard-coded literal: the one attribute JUnit provides for exactly this state was the one value no input could ever move.

SARIF said executionSuccessful: true the same way, on the same run.

Why this is the same defect twice, not a new feature

SkippedCheck exists so "a report that lists only what ran cannot be read as a statement about the whole package", and #104 carried that into both new formats: SARIF toolExecutionNotifications, JUnit <skipped>, with an explicit test that a skip is never smuggled in as a passing result.

A check that started and threw is the same gap arriving by the other door. It reaches the report having evaluated nothing, which is precisely what #61 established when it stopped the engine publishing a crashed rule's partial outcomes ("a check that did not finish did not evaluate anything"). The disclosure was propagated to not_run in both formats and not to this.

What changed

• JUnit — <error> is now reserved for a check that could not complete, symmetrically with <skipped> for one that never started. JUnit already separates "the assertion did not hold" from "the test could not run"; this is the second. errors is counted rather than asserted.
• SARIF — executionSuccessful is read off the report, and each such rule gets an error-level toolExecutionNotification with associatedRule, so the false is diagnosable rather than bare.
• Nothing moves out of sight: the warning stays in SARIF results and in the JUnit case list. Both formats gain, neither substitutes.
• The prose formats are untouched — they already print the sentence.

Finding gains check_completed (default true). The fact otherwise existed only inside a localized English sentence, and neither CI format reads prose; sniffing the title or message would have been both fragile and locale-dependent. Both CI formats stay unlocalized, so no new localizable strings: make i18n still reports 210 messages, parity across en, es, the same 210 as before this branch.

How it was verified

make lock-check lint typecheck security audit i18n all clean (audit did reach the network here and passed). Full suite 554 passed, branch coverage 95.40% against the 90% floor. README's gate count 544 → 554, which test_readme_test_total_matches_what_pytest_collects enforces.

Three negative controls. Each sabotage was read back from disk to confirm it landed before running, then restored from a cp -p byte copy verified by shasum:

sabotage	result
JUnit errors back to the literal "0"	RED — 1 test, on assert suite.get("errors") == "1" (assert '0' == '1')
SARIF executionSuccessful back to True	RED — 1 test, on assert ...["executionSuccessful"] is False (assert True is False)
engine's check_completed=False deleted	RED — 1 test, test_the_internal_error_finding_records_that_the_check_did_not_complete (assert True is False)
both files restored (shasum matches 0b88e864… / ab70ca2a…)	GREEN — 554 passed

The third control is the one worth reading. It fails only the engine test and leaves all eight renderer tests green, because those build findings directly rather than running a rule. That is exactly the absorption this pair of tests exists to prevent: without the engine-level assertion, the renderers could keep their contract while the one producer that sets the flag quietly stopped setting it. The paired positive assertions (executionSuccessful is True on a completed run, errors == "0" on a completed run, check_completed is True on an ordinary finding) are there so none of the three can be satisfied by a constant.

What needs your call

1. The schema bump. report_schema_version goes 1.1 → 1.2. Additive, exactly as 1.1 was for not_run in Name the checks that did not run, in every report format #42: a 1.1 reader that ignores check_completed still reads every report correctly, and diffing.SUPPORTED_REPORT_SCHEMA_MAJOR is "1", so diff compares 1.1 and 1.2 reports without complaint. If you would rather not move the minor for one boolean, say so and I will drop it. Two test assertions carry the literal (tests/test_manifest.py, tests/test_i18n.py).

2. A related gap I did not fix. On that same run, _scope_line still emits "Every check that applies to this filing type ran." whenever not_run is empty — including when a check ran and threw. Defensible as written (the check did start), and the console prints the "could not complete" warning three lines below it, so no console reader is misled. I left it because correcting the wording means new localizable strings, which would put make i18n red until someone writes the Spanish, and the es catalog is already a maintainer draft pending Qualified CEQA and Spanish review of the AI prompts, refusal suite, and corpus #49. Worth a look separately if you want the scope line to mean "ran to completion".

Not touched, so a later pass can pick them up: the diff verb's summary line counts only finding deltas and states no skip counts (the console and HTML both list moved skips in a section below it, so nothing is hidden — only uncounted); and ai … --output takes a bare file path, so pointing it at a directory raises an uncaught IsADirectoryError rather than behaving like check --output / diff --output (#101).

Prepared with AI assistance; reviewed before submission.

[ChelseaKR]

Rebased onto main (e8d390d). This branch was CONFLICTING after #106 merged; it is now clean and still a draft, so the sweep leaves it alone.

Two conflicts, both mechanical, both in prose rather than code:

• README.md — the gate figures. Recomputed rather than picked: 580 tests, and mypy over 39 source files with 221 catalogue messages, which are main's figures since this branch adds neither a source file nor a localizable string. test_readme_test_total_matches_what_pytest_collects was red at 570 until the number was corrected, so the figure is the one pytest --collect-only actually reports.
• CHANGELOG.md — two Unreleased entries wanted the same position. Both are kept, main's first.

make verify clean on the rebased head: lock-check lint typecheck test security audit i18n, the audit reaching the network. 580 passed. No source file changed in the resolution.

Nothing about this branch's substance moved. The report_schema_version question in the body is untouched and is still yours; if you would rather not move the minor for one boolean, the two assertions carrying the literal are still in tests/test_manifest.py and tests/test_i18n.py.

One sequencing note that appeared today. #90 (agency-local rule packs) also proposes report_schema_version 1.2, for an additive rule_packs field. Whichever lands second needs 1.3, or the two land together under one 1.2; two different 1.2 shapes reaching main at different times would make the version unreadable for exactly the consumer it exists for. Recorded on #90 as well.

Prepared with AI assistance; reviewed before submission.