Complete receipt-backed binding compliance and retire legacy claims - #219
Open
evildarkarchon wants to merge 66 commits into
Open
Complete receipt-backed binding compliance and retire legacy claims#219evildarkarchon wants to merge 66 commits into
evildarkarchon wants to merge 66 commits into
Conversation
- Document the Display Content/Layout split and implementation plan - Resolve infrastructure error stage labels through the core vocabulary in the Python CLI
- Add Ruff to Python development dependencies and lockfile - Document centralized wording tests and thin frontend renderer audits
A paused Crash Log Scan Run reports whether Reset To Default can succeed
at all, via `local_ignore_reset_available`. The TUI honoured it; both C++
frontends offered the decision unconditionally. Choosing it spends the
one-shot continuation before core validates it, so the user is left with
no scan, no repair, and no second attempt without starting over. Nothing
on disk is touched, so this is a recoverability gap, not data loss.
Native CLI: the prompt seam could not receive the fact at all — its
callback carried only presentation lines. It now carries
`CliLocalIgnoreRecoveryPresentation { details, reset_available }`, and
`read_cli_local_ignore_recovery_choice` takes the availability. When it
is false the `[R]` line is not printed, the bracketed letters narrow to
`[P/C]`, and `r`/`reset` are rejected like any unrecognized word. The
attempt limit, EOF handling, and cancellation checks are unchanged.
Qt GUI: `ScanRunInstalledYamlDataPresentation` mirrored every
neighbouring DTO field and dropped this one; it now projects
`localIgnoreResetAvailable`. The prompt typedef, the controller hop, and
`promptLocalIgnoreRecoveryChoice` gained a `resetAvailable` parameter,
and the reset button is not created when it is false. The null guard on
`clickedButton() == resetButton` is load-bearing: both are null when the
box is closed without a click, so without it a dismissal would report a
durable reset the user never asked for.
All three frontends treat absent Installed YAML Data as available.
Silence is not a denial, and withdrawing an option on an unknown answer
would regress the behaviour that shipped before the fact existed. The
GUI resolves it in one place, `offersLocalIgnoreResetToDefault`, because
`localIgnoreResetAvailable` defaults to false to mirror the bridge DTO
and a site that skipped the presence check would fail the other way.
Tests: the CLI case that pinned the reset option as unconditional is
split into an available case and an unavailable case; a Qt case asserts
the button is absent, that abandonment still cancels, and that the
remaining decisions still work. The end-to-end cases assert availability
survives Rust, the bridge, and the Qt projection in a real run. Both
new cases were confirmed to fail against the restored bug.
Wording matches the TUI's existing sentence verbatim rather than being
phrased afresh, so the presentation crate has one wording to adopt
rather than three to reconcile.
No Rust, core, or binding surface changed; all three parity baselines
are untouched. Verified with `classic-cli/build_cli.ps1 -Test` (71 CTest
cases, 24 integration scenarios) and `classic-gui/build_gui.ps1 -Test`
(21 tests).
Closes #171
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CrashLogScanRunStatus, ScanProgressPhase, LocalIgnoreRecoveryDecision, and InstalledYamlDataRole now implement the Vocabulary naming contract. Tokens are byte-identical to what each surface already published; only Display Labels are added. Closes #172. Labels follow the glossary rather than any one frontend. CONTEXT.md already defined a run status as "completed, no Crash Logs found, Local Ignore recovery required, setup failed, cancelled before discovery, or cancelled after discovery", so those six are recorded rather than invented -- which is why Cancelled reads as "cancelled after discovery": its bare token cannot be told apart from CancelledBeforeDiscovery, and telling them apart is why both variants exist. The phase labels settle the widest divergence of the four, the TUI's participles against the GUI's nouns, in favour of the only form that reads grammatically in the progress sentence both build around it. InstalledYamlDataRole is the one adopter whose forms differ by case alone. Its Display renders the *label*, not the token, because every consumer splices it into prose; routing it through as_str would have silently downcased Main. ResumeErrorKind deliberately stays token-only. It is a stable error code, not a name a person reads, and the reason is recorded on the type. The TUI's naming table for the progress phase is gone -- the four participles now resolve through label(), output byte-identical. Node and Python delegate their duplicated token tables to the core, and three hardcoded expectation arrays in their projection tests became VARIANTS loops. Node keeps publishing the core token unchanged rather than through js_token: the scan-run surface's tokens are snake_case, and camelizing one would rename a value consumers match on. The TUI audit covers two of the four. CrashLogScanRunStatus and LocalIgnoreRecoveryDecision sit in a new DEFERRED_ENUMS constant with their reasons -- the first is still composed into count-bearing sentences only the presentation crate can absorb, the second has no table at all and trips only arm_body_end's documented over-read. A meta-test asserts every adopter is either audited or explicitly deferred, so the gap is checked data rather than a comment that rots. Two baselines move by exactly two rows plus a timestamp: replacing the inherent as_str on two enums deletes entries the Node and Python surface parsers see but trait members are invisible to. parity_contract.json, both surface inventories, and both coverage-registry hashes are untouched, which is the shape binding-parity-policy.md documents for this case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gate condition 4 of the Local Ignore Recovery Phase asked whether the Qt GUI can present a recovery prompt driven by core-supplied decision descriptions without blocking its event loop. It can, and the path is the one the GUI already ships: Qt::BlockingQueuedConnection parks the scan worker thread, not the GUI thread, and QMessageBox::exec() starts a nested loop on the GUI thread rather than suspending it. The two existing tests each cover one half of that seam. The dialog test calls the real modal directly on the GUI thread with no worker, and the controller test drives the real worker-thread marshalling but answers with a stub and pumps the GUI thread by hand -- which would mask exactly the freeze in question. This joins them and measures the loop with a heartbeat timer, a sibling window's repaints, a call queued from inside the nested loop, and synthesized input, all while the GUI thread sits in an ordinary QEventLoop::exec(). Four cases isolate the controller seam; two more run a real ScanWorker on a real thread against a genuinely malformed Local Ignore, so the legs that only reason could reach -- taking the one-shot continuation before the prompt, and the block_on resume after it -- are executed rather than argued. Abandoning and choosing a decision are exercised on both paths. The finding records the five constraints this fixes on the shared prompt shape: the call stays synchronous and value-returning, its payload stays copyable and carries no continuation or rust::Box, rendering happens on the worker thread before the hop, the answer stays a plain enum, and the same-thread short-circuit in makeLocalIgnoreRecoveryPrompt must survive because BlockingQueuedConnection self-deadlocks without it. The prompt shape the brief proposes satisfies all five. The CLI and TUI are unconstrained by any of it. Refs #173, #170. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rust now owns Crash Log Scan Run Display Content. The new crate turns a run result, a run event, an infrastructure error, or a resume error into an ordered sequence of display lines, each a severity plus typed segments, so a frontend can style a path as a link or emphasise a number without re-deciding the words around it. Nothing consumes it yet, by design: the wording ships pinned first so the frontends that follow have something fixed to render. Core never interpolates. Counts, paths, and names stay typed payloads, and a count arrives with its noun already agreeing with its value. Agreement is structural rather than remembered -- CountedNoun exposes only for_value(), so a renderer names a noun pair and the value picks the form. Writing "1 logs" is not a bug the tests catch; it is one the type will not let you write. The crate is deliberately separate from classic-scanlog-core rather than a module inside it: that crate's scan_run and scan_run::contract already form a cycle, and presentation placed inside would deepen it. As a crate, Cargo enforces the one-way edge. It also depends on classic-config-core, because the scan-run contract exposes Installed YAML Data types it does not re-export -- the same edge classic-tui carries for the same reason. Two renderers take typed facts rather than the structs carrying them, because InspectedYamlDataFile and InstalledYamlDataRunDiagnostic are accessor-only with no public constructor. Shaping them that way pins the Installed YAML Data block without widening classic-config-core's API for a test. Out of scope and deliberately absent: the Local Ignore recovery prompt, which lands with the gated recovery phase, and the FCX Mode setup projection, whose four types have not adopted the shared vocabulary. Adds docs/api/classic-scan-presentation.md, indexed and placed in the layering rationale. Refs #174 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Render scan results and events from Rust-owned display segments - Preserve TUI-specific layout, recovery handling, and audit coverage
Severity now reaches the status row, not just the two overlays. That row is written from around sixty places across app.rs, event.rs, and the three workflow modules, and all but a handful carry no severity at all -- so App stores the severity paired with the exact text it applied to. Any other write silently invalidates it, which keeps a failed run's red off the unrelated message that replaced it without asking sixty call sites to remember to clear a flag. The recovery overlay stated why the run paused three times over: its own title, core's status line, and core's Message: line. The copy this frontend was making is gone, along with the invented fallback sentence behind it -- when a paused run carries no message the status row falls through to core's rendered status instead of to prose written here. classic-vocabulary moved to dev-dependencies. No non-test code in the crate calls label() any more, because every label the TUI shows arrives inside a DisplaySegment::Label, and that is the clearest single signal the migration worked. Also restores a test rationale that outlived the code it described, replaces a byte slice that would panic on a multi-byte sentence-initial character, and strengthens the resume-failure assertion from "not empty" to core's own headline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The native CLI now describes a run in the same words the TUI does, because both render the display lines classic-scan-presentation produces. C++ never holds a Rust run result, so the bridge renders while the Rust value is live and carries the lines across as mirrored data. The continuation is still taken out of the result first, which is now load-bearing rather than incidental: the render entry point borrows, so moving it out afterwards would not compile. Observed events are rendered inline on the observer callback, before the event reaches C++. cxx cannot express a payload-carrying Rust enum, so a segment crosses flattened as a ScanRunDisplaySegmentKind tag plus text, path, and count fields, with the fields the kind does not use left empty and a count's Rust-resolved noun in the text field. execute and resume return the same ScanRunContractExecutionResult, so one display_lines field covers both entry points and all three payloads rather than four fields saying the same thing. The seven scan_run_*_label entry points are unchanged; they remain correct for labelling an enum outside a display line, and the Qt GUI still uses them. The CLI keeps section ordering, exit codes, which event kinds earn a durable line, and which stream a line reaches. Its output stays plain and pipeable: the per-segment styling is deliberately the empty choice for every kind. What it loses is the ability to invent or reword what the run says. Its event rendering migrated too, beyond the ticket's acceptance list, because leaving it composing sentences would have kept exactly the drift this removes. Wording is pinned once, in the presentation crate. The CLI tests became renderer conformance over deliberately unreal words, and the display-label audit inverted: the CLI calls no label accessor now, so their absence is asserted and the positive half moved to "the renderer reads every kind". Refs #176 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects surfaced by the display-line work, none introduced by it. InstalledYamlDataRole's Display Label is asymmetric on purpose: Main names a glossary domain term while game is the ordinary adjective in "selected-game YAML Data", and the lowercase form is load-bearing in "no usable Installed YAML Data source for game". The presentation crate put it at the start of a line anyway, so "game YAML Data: bundled" sat under "Main YAML Data: bundled" and read as a capitalization bug rather than the distinction it is. The sentence now opens on fixed prose - "Selected Main/game YAML Data:" - so the label is never line-initial, and a test pins that rather than the wording. The label itself is untouched, being correct everywhere it was designed to appear. This affected the TUI as well as the CLI. The parity parser's name scan is a keyword regex over raw text, so a doc comment merely mentioning `enum Foo` became a contract row. Three phantoms had reached the committed baseline: definitions twice, from a comment reading "cannot share enum definitions", and mirroring. Harmless until somebody reworded the sentence, at which point a bridge that had not changed reported drift - which is exactly what happened while adding the display types. Comments are now stripped before the scan. Three rows removed, no real row added, changed, or dropped. The comments contorted to dodge the bug are restored to their natural wording, and the gate stays green on them. --update-baseline mirrored the committed contract straight back, so it could never accept an added, removed, or modified item: the contract returned unchanged and the next run reported the same drift. The flag only refreshed reports, while the contributor guide documented it as the one-step way to accept an intentional bridge change - the two-step bootstrap was the only path that worked. It now rewrites the contract from current source, reports what it accepted, and exits 0. The committed reports are recomputed against the contract they ship beside, or the next run would read the fresh baseline as stale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`CrashLogScanRunContinuation::abandon` cancels a paused Crash Log Scan Run and then claims its continuation with a decision the run never acts on. `resume` checks cancellation immediately after the one-shot claim and before the retained recovery plan is consumed, so neither Proceed Without Ignore nor Reset To Default is ever applied: no backup, no publication, and the malformed Local Ignore file is left exactly as it was. The caller gets the ordinary post-discovery cancelled result. `LocalIgnoreRecoveryDecision` keeps its two variants. A third would be the honest domain fix, but it reshapes a type crossing five binding surfaces and reopens the scan-run ADR's cancellation semantics, so this operation encapsulates the workaround instead. The TUI's dismissal path now routes to `abandon` rather than picking the placeholder decision itself. The `cancel()` moves from the UI thread into the spawned task; nothing reads `is_cancelled()` on that control in between, and the only other consumer just calls `cancel()` again, which is monotonic. Contract tests pin the cancelled result, single consumption, replay through both seams, and an untouched filesystem — compared as whole trees, so a stray Autoscan Report or Local Ignore backup fails as a readable path diff. The shared cancelled-after-discovery assertions are extracted so the pre-cancelled `resume` path and the `abandon` path cannot drift apart. `abandon` is Rust-only for now: it is a new inherent `pub fn`, so the Node and Python `rust_api_surface.json` baselines are refreshed, leaving both `parity_contract.json` files untouched. Exposing it across the bindings and adopting it in the native CLI and Qt GUI is issue #181. Closes #177 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both tests claimed to cover concurrent replay, and neither did. `tokio::join!` polls both futures on one task, and the claim inside `resume` is a synchronous `take()` under a mutex with nothing awaited before it — so the futures always resolved in poll order, the mutex was never contended, and "exactly one succeeded" could not have failed. `race_two_claims` runs the two claims on real OS threads released together by a barrier. Verified parallel: with a 400ms sleep injected per thread the test completes in 0.41s, not 0.8s. The barrier also makes the arrangement self-verifying, since `Barrier::wait` cannot return until both threads arrive — if the two ever stopped overlapping this would hang rather than quietly pass. The helper's docstring bounds what this proves rather than overstating it. `Option::take` under a `Mutex` makes two winners unreachable for any lock discipline that compiles, so this is not a test a non-atomic claim would fail. It proves the contract holds end to end under real parallel entry: one winner, the loser gets the typed consumed error rather than a wrong-typed one, and neither thread deadlocks or panics. `proceed_without_ignore_continuation_rejects_every_replay` had the same defect and gets the same fix; leaving it would have left the identical false claim sitting next to the corrected one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Qt GUI now describes a run in the same words the TUI and the native CLI do, because all three render the display lines classic-scan-presentation produces. A diagnostic a user learned to read in one frontend is readable here, and a wording fix reported once reaches this window too. No bridge surface was added: this phase reuses the DTOs #176 landed, so no parity baseline moved and no gate had to be regenerated. What the GUI kept, all of it Display Layout: ScanRunTerminalKind and its one-to-one mapping onto lifecycle signals, the counts, the discovery-ordered logs, the FCX Mode setup projection grouped in after the rendered lines, and the run-level Installed YAML Data warning's decision to interrupt. What it gained is styling: severity to a dark-theme colour, Count to an emphasised value beside Rust's noun, Emphasis to italics, and Path to a file: anchor that showScanRunMessage makes clickable with Qt::TextBrowserInteraction. The terminal signals that carry prose carry rich text, because all three end in a QMessageBox. The progress row cannot: setStatusMessage writes a QProgressBar format string, so scanRunStatusLine reduces the block to its leading line, which is safe because every render entry point opens on the line stating the outcome. The signal plumbing itself is untouched, invariant errors for a missing continuation and a missing prompt included. Event rendering migrated too, beyond the ticket's acceptance list, for the reason the CLI's did: leaving eventStatus composing "Found N crash logs" would have kept exactly the drift this removes in a frontend whose event DTO now carries the rendered lines. The per-log outcome line moved out of the worker with it — three composed shapes replaced by logging the rendered run once, and ScanRunLogPresentation::failures went with them. Five of the seven bridge label accessors lost their last GUI caller. The two that survive both label an enum outside a display line, in the one-row YAML Data status suffix that has no room for a rendered block. test_display_label_audit inverted for the other five and asserts their absence; its positive half moved down a level to "the renderer reads every segment kind and every severity". Per-log disposition stays unlabelled here, as since #167: presentLog still maps the three variants onto booleans and invents no results-view column. The Installed YAML Data warning keeps its policy and loses its prose. Its body is the run's rendered lines under a GUI-owned header, because Rust exposes that block only through render_run_result and picking it back out by position would be a structural assumption about a sequence that carries no structure. Which lines it withholds is preserved exactly, matched on a diagnostic message payload the GUI already holds typed rather than on prose. formatScanRunRejections is the one sentence about a run this frontend still composes. The event's rendered lines state the rejection count but not the per-rejection path and reason, which the GUI shows at discovery time; migrating it needs a core change and belongs with the Node/Python phase. Refs #178 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #179. A Node consumer now receives what a run says, in Rust's words, alongside the projected result and on every observed event, so a frontend built on this binding speaks with CLASSIC's voice instead of reimplementing its wording. This is the first surface with no frontend of its own. It renders nothing and styles nothing; it carries the lines and hands them to whatever a consumer builds. That made the flattening decision easy: reuse the C++ bridge's shape unchanged -- a kind tag plus text, path, and count, with the fields the kind does not select left empty rather than absent -- rather than an idiomatic one with optional payloads, because a consumer reading two bindings would otherwise read the same segment two ways. Three fields, not one. The bridge has a single execution envelope with presence flags, so one display_lines covers all three payloads. Node resolves two envelopes and rejects a resume error rather than returning it, so the same coverage costs displayLines on JsScanRunSuccess, on JsScanRunFailure, and on the rejected error beside its stable code. The envelope pair is shared by scanRunExecute and scanRunResume, which keeps the initial run and the continuation resume on one field each. Rendering the rejected resume goes beyond the ticket's acceptance list, which named only the two results and the event. The bridge renders resume errors, and leaving them out would have left a Node consumer composing its own sentence for a reset conflict -- the exact drift this work removes, in the place where wording matters most. It is purely additive. The replay rejection was routed through the shared builder on the way. ScanRunResumeTaskOutput::ContinuationConsumed used to construct its own code and message by hand; both strings were already byte-identical to ResumeErrorKind::as_str and ResumeError's Display, so the pair recorded the agreement instead of checking it -- and it is what would have left that one rejection with nothing to say. It gains kind and displayLines and changes neither the code nor the message; a test pins both as literals. The two hand-written ts_arg_type observer unions had to move with the struct. They narrow JsScanRunEvent to the payloads each kind carries, which the flat all-optional struct cannot express, and they are a second copy that no compiler checks against the Rust. The CLI build caught the disagreement. Wording stays pinned once, in classic-scan-presentation. No test here restates a sentence: the Rust tests pin the flattening -- every kind fills exactly its own fields, segments keep their order, every severity has a distinct twin, a count carries the noun Rust agreed with, every resume variant keeps its code out of the prose -- and the Bun and Node suites prove the same facts across a real run, plus the one thing a unit test cannot: that every Autoscan Report the run wrote arrives as a whole Path segment. Only node_api_surface.json moved in the baseline. No Tier-1 row changed, because the parity contract maps Tier-1 core crates to Node exports and classic-scan-presentation is not one of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ce markers
Two follow-ups to the Node display-line work, both of the same shape: a second
copy of something, drifting from the thing it copied.
## The Node demo CLI composes its own sentences no longer
`node-bindings/classic-node/cli/run-scan.ts` is a frontend on the binding rather
than part of it, so no ticket named it. It was migrated anyway, because leaving
it would have left a live `plural(count, "log", "logs")` sitting directly on a
surface that had just started carrying Rust's already-resolved nouns -- one
import away from the thing that removes it.
It renders exactly as the native C++ CLI does: segments concatenated in reading
order with single spaces, no styling, no capitalization rule, paths whole.
Severity is spent entirely on the choice of output stream, cut at Warning for
the same reason -- a run paused awaiting a Local Ignore decision carries that
severity and belongs on stderr.
The sharpest thing this fixes is a raw token printed as prose. The
infrastructure failure path read `${error.stage}: ${error.message}`, so a user
was told the run failed during `formid_database_access`. That is the same bug
#170's problem statement records against `classic-py-cli`, surviving in a second
frontend. It is fixed the same way, and `error.stage` still carries the token
for anything matching on it.
Three invented fallback sentences went with it -- a setup-failure headline, a
two-clause explanation of what Local Ignore recovery means, and a no-logs line
that named directories from `process.cwd()` and the configured scan path rather
than the ones discovery actually searched. The completed summary lost `Scanned`
and `Errors`, exactly as the native CLI's did, and keeps `Reports`, `Failed`,
`Duration`, and `Speed`: the two aggregates over per-log outcomes the contract
does not tally, and the two facts from a clock it does not carry.
JSON mode keeps its shape and takes on no Display Content. Which event kinds
earn a console line is unchanged. The two display enums are `const enum`, so
tsc inlines them and erases the import -- load-bearing, because this command
resolves the binding at run time and must not gain a static require of
`../index.js`.
Wording assertions were re-anchored on paths and exit codes the payload carries,
matching the call the native CLI's end-to-end tests made. Renderer conformance
moved down a level into three tests over fabricated lines. Two assertions were
added for the drift itself: the summary must not print `Scanned:` or `Errors:`
again.
## Two stale evidence markers in the shared scan-run manifest
`scan-run-contract-variants` was failing, blocking, before any of this work
started. `presentations.rust_owned_facts` named four test identifiers by string;
the CLI's two were deleted by the CLI display-line migration and the TUI's two
by the TUI migration, both of which renamed the tests around what those
frontends now do. The check fails fast, so only the CLI pair was visible.
Both facts are still covered under new names, so the markers are re-pointed
rather than the tests renamed back: `an_event_renders_cores_lines_in_cores_order`
and `per_log_lines_stay_in_discovery_order` for the TUI, and the matching
"stated in Rust's words" / "never reorders" pair for the CLI.
Worth naming the shape: a marker list is a copy of a test's identity living in a
JSON file, and it rots exactly like the per-frontend naming tables this epic
deleted -- silently, because a renamed test still passes.
`python tools/binding_compliance/check_compliance.py --profile full` now passes
20/20 with the one documented non-blocking CXX runtime-coverage gap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #180. The last of the render migrations. A Python consumer now receives what a run says, in Rust's words, and `classic-py-cli` states a Crash Log Scan Run in the same sentences as every other frontend. ## The binding `ScanRunDisplayLine` and `ScanRunDisplaySegment` mirror the C++ bridge's flattening unchanged -- a kind tag plus text, path, and count, with the fields the kind does not select left empty rather than absent -- because a consumer reading two bindings would otherwise read the same segment two ways. One field, not three. Python's `ScanRunExecution` has result/error presence like the bridge's single envelope, and `scan_run_execute` and `scan_run_resume` return the same type, so one `display_lines` covers the initial run, the continuation resume, and both payloads. Node needs three only because it resolves two envelopes and rejects a resume. The two tags are snake_case token strings rather than pyo3 enums. Every other output tag this surface publishes -- a run status, an event kind, a log disposition, a scan phase -- already is one; the surface's only pyo3 enum is an input. Parity is the flattening beneath the tag, not its spelling per language. `path` is a `str` because every other kind must leave it empty and an empty `Path` is `Path('.')` -- a wrong value rather than an absent one. The five resume exceptions carry lines too, beyond the ticket's acceptance list, for the reason Node's rejected resume does: leaving them out would leave a consumer composing its own sentence for a reset conflict. The replay rejection was routed through the shared builder on the way; `scan_run_reset_error_to_py` became `scan_run_resume_error_to_py` and absorbed the arm the entry point built by hand. Both hand-written strings were already byte-identical to `ResumeErrorKind::as_str` and `ResumeError`'s `Display`, so the pair recorded the agreement instead of checking it. It gains `kind` and `display_lines` and changes neither the code nor the message; a test pins both as literals. Only `python_api_surface.json` moved in the baseline, mirroring the Node phase. ## The CLI `classic-py-cli` is where #170's raw-token bug lives: `f"...failed during {stage_label}: {message}"` told a user a run failed during `formid_database_access`. It is gone, and `error.stage` still carries the token. Four invented sentences went with it. The four terminal branches collapsed into a status-to-exit-code table: the exit codes are this frontend's and are unchanged, the prose beside them no longer exists here. The Installed YAML Data block appears for the first time, arriving inside the rendered sequence. `_infrastructure_error_stage_label` is deleted with its docstring, and with it this CLI's only call into the six label accessors. Severity reaches no surface. This CLI writes one stream of plain lines through an output envelope every command shares and the compliance harness reads structurally, so routing by severity would change that shared contract rather than this run's presentation. Mapping every severity onto plain text is explicitly correct; the severity stays on the line. JSON mode is untouched in shape. Where `error.message` fell back to a sentence written here it now takes the run's leading rendered line, so that field is Rust's words in every branch. One sentence stays this CLI's: a binding that hands back a run and says nothing about it is a broken binding promise, not something a run said. ## The audit The audit this frontend never had, which is the direct reason the token bug survived here after the same fix landed in the TUI. It ports the TUI's coverage test and the C++ audits' inverted accessor-absence assertion, and adds the sentence-template detector the brief has owed since step 2 -- enforceable only now that every frontend has stopped writing those phrases. Three things it does that are worth copying. It covers all four ways Python builds a string, so a contributor reaching for `+` instead of an f-string does not pass. Its coverage test walks recursively, so a future subpackage cannot escape -- the exact failure it exists to prevent, in the one shape a top-level glob would not see. And each detector is run against the drift it catches, because an audit whose detector never fires reads as coverage while providing none. Comments and docstrings are excluded from the literal scans, for the reason the CXX parity gate's name scan was fixed: a comment describing the drift is not the drift. No test restates a sentence. Wording stays pinned once, in `classic-scan-presentation`. The Rust tests pin the flattening; the Python suite proves the same facts across a real run plus the one thing a unit test cannot, that every Autoscan Report the run wrote arrives as a whole `path` segment. The segment taxonomy held: six kinds, unchanged, across all three baselines. Validation: 524 pytest, 38 crate tests, `cargo fmt`/`clippy --workspace` clean, stub validation 17/17 with warnings as failures, Tier-1 Python parity gate green, CXX parity gate green, binding compliance full 20/20 with the one documented non-blocking gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comments and docs only; no behaviour, no surface, no gate moves. The reasoning left behind for `ScanRunContinuationConsumedError.kind` was the weak half of it -- that the shared builder made the field free. That reads as an accident kept for convenience, and a reviewer read it exactly that way. The reason that holds is Python-internal: the four reset exceptions beside it have published `kind` since they were written, so this was the one member of the resume family for which `except (...) as error: error.kind` raised `AttributeError`. The builder made the field free; the family's existing shape is what makes it correct. Also names what is explicitly *not* the reason. Node publishes the same property on its own resume rejections, but binding surfaces answer to the core's contract, not to each other's shapes -- `node-python-contract-map.md` says so outright. Citing cross-binding symmetry as justification would invite the next contributor to widen one surface because another one is wider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…h native frontends `CrashLogScanRunContinuation::abandon` was Rust-only: the TUI called it directly while the native CLI and the Qt GUI each wrote the cancel-then-resume-with-a- placeholder sequence it encapsulates. Both now route their cancel choice through the shared operation, and it reaches all three binding surfaces so a consumer can abandon a run the same way. - CXX: `scan_run_continuation_abandon`. Unlike `scan_run_continuation_resume` it does not throw -- resume is fallible only because it must reject an out-of-range recovery decision before claiming the one-shot continuation, and abandonment takes no decision. A `throws` contract that can never fire would force callers into unreachable handling. - Node: `scanRunAbandon`. Python: `scan_run_abandon`. Both model abandonment as the absence of a decision rather than a third variant, and share one claim path with their `resume` sibling so the two cannot drift. `LocalIgnoreRecoveryDecision` still has exactly two variants everywhere. - Both frontends map the choice through an exhaustive switch producing an optional decision, so a choice added later trips -Wswitch rather than silently resolving to Proceed Without Ignore. Neither frontend cancels first any more; the shared operation cancels the control itself, strictly before the claim, which is the ordering each hand-written copy had to get right independently. Tests assert what a user would notice: the run reads as cancelled with its retained discovery intact, the control is left cancelled, the malformed Local Ignore and the absent backup directory are untouched, no report is written, and the shared claim rejects a later abandon and a later resume alike. "An abandoned run emits nothing" is pinned from C++, because `ScanRunObserver` is a C++ virtual class no Rust-side bridge test can implement. The pre-existing end-to-end cancel pins in both frontends pass unchanged. All three parity gates green against regenerated baselines (+1 row each, purely additive). C++ tests run through the build wrappers; Python tests against a built virtualenv. Closes #181 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four frontend display-label audits gain a sentence-template detector over a deny-list of the domain phrases classic-scan-presentation owns. The vocabulary work centralized names and stopped there, so the sentences around them drifted apart again; this is what stops the same rot after the presentation consolidation. The deny-list lives in one file rather than four inline copies. A per-frontend list would put the four-copies drift back into the test layer one level up from where this work removed it: adding core-owned prose would mean remembering four lists, and forgetting one would leave the phrase unenforced in exactly the frontend nobody was watching. The detector is deliberately narrow. A general "no format strings" rule would bury real findings in false positives and get switched off. No per-frontend golden wording suites are added either; wording stays pinned once, in the presentation crate. Each audit carries two guards, because a detector that cannot fire reads as coverage while providing none: one asserts the shared file is readable and non-empty, the other feeds the detector the drift it exists to catch alongside a compliant rendering loop. The native ports search comment-stripped source, so they handle raw strings, a `//` inside a literal, and a character literal holding a quote -- each of which would otherwise drop the rest of a file from the audit silently. Two gaps are recorded rather than implied: - `succeeded,` is core-owned but stays a named local supplement in the Python CLI audit, which enforced it before the shared file existed. The Qt GUI still composes its own status-bar summary from the scan worker's `finished` signal, so a shared entry would fail that frontend for a migration that has not happened. - node-bindings/classic-node/cli/run-scan.ts renders these display lines and has no audit at all. It is named in the deny-list header so the deficit is data a reader can find, not silence. Verified by reintroducing a locally written sentence into each of the four frontends in turn and watching its audit fail, then reverting. C++ suites run through build_cli.ps1 -Test and build_gui.ps1 -Test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ode CLI
Three changes that turned out to be one.
The Qt GUI's status bar was the last surface composing its own sentence
about a Crash Log Scan Run:
Scan completed: %1 logs scanned in %2s (%3 succeeded, %4 failed)
It re-derived the plural of "logs", so a one-log run read "1 logs" there
while every other frontend read "1 log". The cause was upstream of the
window: ScanWorker::finished(int, int, int) was the one terminal signal
carrying no words, so the window had nothing to state the outcome with.
It now carries the rendered run like the other three, and onScanCompleted
collapses onto the shape onScanCancelled already used -- the run states
its outcome, the window adds only the elapsed time, from a clock Rust
does not carry, and the Installed YAML Data suffix. The counts stay on
the signal because consumers key on them as data.
That unblocked `succeeded,`, which core owns and which had lived as a
named supplement in the Python CLI's audit because a shared entry would
have failed the GUI for a migration that had not happened. It is now an
ordinary entry and the supplement is gone.
Enforcing it exposed that the source-scanning detectors read too much.
They stripped comments and searched the remaining code, so `succeeded,`
matched the GUI's own `emit finished(terminal.succeeded, terminal.failed,
...)` -- a line that says nothing to a user. They now extract string
literals, newline-separated so two adjacent literals cannot fuse into a
phrase neither contains. That removes the whole class of false positive
and matches the Python audit, which reads literals off the AST for the
same reason. The stopgap rule that every entry contain a space is gone
with it, and each audit records why.
The Node CLI gains the audit it never had. run-scan.ts had already
migrated and composed no sentences of its own; what it lacked was
anything keeping it that way -- the same unenforced state the Python CLI
was in when a raw Vocabulary Token survived there. It carries the
deny-list detector and the coverage meta-test; cli.spec.ts already held
the renderer-conformance half. The roster went from four audits to five
without a detector changing, which is the property the shared deny-list
file was chosen for.
Verified by injecting the phrase into the GUI and watching its audit
fail, then reverting. Full suites: Rust workspace, native CLI 82 + 24
integration, Qt GUI 22, Python 526, Node 1043. C++ through
build_cli.ps1 -Test and build_gui.ps1 -Test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ty attached `render_local_ignore_recovery` produces the content of a Local Ignore recovery prompt: the display lines explaining the pause, plus one description per recovery decision stating its label, what choosing it will do, and whether this run can honor it. Availability travels attached to the decision it describes rather than as a separate flag beside the prompt. That closes a confirmed gap by construction: a consumer cannot offer an unavailable decision without ignoring data placed directly in its hands. `local_ignore_reset_available` previously sat one level away on the Installed YAML Data DTO, and two native frontends never read it. The prompt crosses all three binding surfaces with the same segment flattening as the rest of Display Content: `ScanRunRecoveryPrompt` behind `has_recovery_prompt` on CXX, `JsScanRunSuccess.recoveryPrompt` on Node, and `ScanRunExecution.recovery_prompt` on Python. `LocalIgnoreRecoveryDecision` still has exactly two variants; backing out stays the absence of a decision. The decision list is built by walking `LocalIgnoreRecoveryDecision::VARIANTS`, so it can neither offer something the contract will refuse nor omit something it accepts, and the exhaustive matches behind it stop the crate compiling if a third variant is added. This lands the core and transport half of #183; the frontends adopt the descriptions in the change that follows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four frontends now render the Local Ignore recovery prompt's display lines and decision descriptions from core. A user gets the same explanation of what each decision does everywhere, and is only offered decisions that can actually succeed -- not because each frontend remembered to check a flag, but because the availability fact arrives attached to the decision itself. Each frontend keeps its own affordance, and only its affordance: - The native CLI keeps its bracketed letters, input loop, attempt limit, and end-of-input handling. The menu, the offered letters, the retry hint, and the accepted answers are now built in one pass over the decision list, so none can advertise what another withheld. `CliLocalIgnoreRecoveryPresentation` carries `decisions` in place of `reset_available`. - The TUI gates both decision keys on `RecoveryDecisionDescription::available` rather than a hand-read flag. Enter remains deliberately unbound, so no keypress can authorize a durable reset by reflex. - The Qt dialog builds one button per available decision, labelled with core's Display Label, with the descriptions under core's question. The window stays responsive: `BlockingQueuedConnection` parks the worker thread while `exec()` runs a nested loop on the GUI thread. The prompt payload is projected on the worker thread and carries no `rust::Box`, which is what makes that hop legal. - The Python CLI renders the prompt without prompting. A paused run stays terminal there, but it is no longer unexplained: the lines and every available decision reach both the plain stream and `data.recoveryPrompt`. `offersLocalIgnoreResetToDefault` is gone. Its "silence is not a denial" rule was one of three copies; `render_local_ignore_recovery` takes the optional Installed YAML Data so the rule is written once. The three sentences join `core-owned-phrases.txt` with this change rather than before it, which is the only point at which all five frontend audits can enforce them. Closes #184 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`render_recovery_prompt` pushed `render_local_ignore` into the prompt's lines, and `render_run_result` pushes the same line into the Installed YAML Data block. Every surface that receives a prompt receives the rendered run on the same envelope, so all three interactive frontends showed the malformed file's identity twice once they started rendering the prompt -- the drift this crate exists to remove, merely relocated into core. The prompt now states the question and nothing else: why the run paused, plus a notice naming any decision being withheld. The identity is stated once, by the run render, which is where it was already correct. That reduces `render_recovery_prompt` to a single `bool`, and the "silence is not a denial" rule stays where it landed, in `render_local_ignore_recovery`. Also moves the recovery tests out of `lib_tests.rs` into `recovery_tests.rs`, declared from `recovery.rs` with the path attribute, per AGENTS.md rule 11. They run as `recovery::tests::*` now. The corpus in `lib_tests.rs` still renders the prompt, which is deliberate rather than a leftover duplicate: the wording is pinned beside the module that writes it, while the corpus is what subjects the decision descriptions to the crate-wide placeholder and grammatical-number invariants that no per-module file can apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Add ADR-0008 for executable cross-adapter parity evidence - Specify scenario packs, receipts, coverage, CXX integration, and migration gates
Extend the independent compatibility oracle and all semantic adapter runners with preview, caller-controlled commit, rejection, preservation, and stale revision scenarios. Reject known-stale core commits before lock creation while retaining the locked recheck. Keep User Settings conformance in shadow and preserve existing blocking evidence.
Exercise planning, reversal, application, exact backup verification, and receipt-authorized restoration across Rust, CXX, Node, and Python using the independent compatibility oracle. Declare optional empty conflict locks in scenario normalization and retain existing shadow evidence. Remove the unused PyO3 test import that broke workspace CI.
Narrow migrated registry claims after blocking conformance validation while retaining unmigrated loaders and diagnostics. Delegate fixture coverage metadata to receipt requirements and remove duplicate positive assertions. Fixes #211
- Document executable evidence boundaries for both conformance families - Add Pyright to the Python binding development dependencies
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 694ae4d0a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Architecture review*.html | ||
| Architecture review*_files/ | ||
| .mcp.json | ||
| .pytest-tmp/ |
There was a problem hiding this comment.
This rule does not match the five .pytest-215-* trees added by this commit. Those trees contain 477 nested repositories recorded as gitlinks but no .gitmodules mappings; on the reviewed tree, git submodule status --recursive exits 128 with fatal: no submodule mapping found, so the submodules: recursive checkouts in ci-cpp.yml fail before the native builds start. They also add over 142 MB of temporary blobs. Remove these generated trees and ignore the actual basetemp naming pattern.
Useful? React with 👍 / 👎.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Testing
cicompliance profile, and full receipt aggregation.