fix: design-review sweep — doc/code drift, strict-gate coherence, doctor core floor by t41372 · Pull Request #49 · standard-voice/standard_asr

t41372 · 2026-07-26T11:14:43Z

Closes out the 2026-07 full design review: one full-context reviewer over the entire codebase + all open issues/PRs, findings individually verified before fixing, followed by successive independent adversarial review rounds (each round's findings verified then fixed in-PR; review ongoing). Squash-ready.

Blockers fixed

streaming.md taught a nonexistent API four ways (feed_bytes, end_audio() after feed() which raises, deadlines "emit done" when they emit terminal error, error text location). Rewritten against the real API.
StreamDeadlines silently swallowed unknown fields (the only closed input model without extra="forbid"), and the docs taught the wrong field name — the documented example ran with its deadline silently disabled.

Majors fixed

Strict candidate-language rejections now raise UnsupportedFeatureError(param="candidate_languages", mode=...) per spec §RT R2 — REST 422 (was 500 "Internal transcription error"), WS unsupported, CLI exit 2 (was 1). Regression-tested end to end through the real server catch chain.
recommended_wire_format() — the documented first step of streaming — joins the StandardASR protocol, compliance's unconditional required methods, and spec §3.1.
Timestamp-less streaming results (design superseded in the post-review hardening below): Segment.start/end are now nullable — the engine's measurement is stored verbatim (no fabricated 0.0 spans, no reserved extra marker), timestamp_status derives from the values, and to_srt/to_vtt take an explicit unrenderable-cue policy — final API on_unrenderable (UnrenderablePolicy), covering unmeasured spans AND measured spans that quantize to zero on the output grid; default "error" raises SubtitleRenderingError, "omit"/"collapse" are caller choices. The reducer still attaches the aggregate segment_timestamps_unavailable diagnostic.
doctor ignored the core's own numpy floor — a plugin conflicting only with core read as clean. Core now joins the analysis; plugin-vs-core incompatibilities get a dedicated conflict (isolation cannot help — core is in every worker) and are excluded from the environment-level analysis whose remedy is isolation.
Compliance remediation message named a nonexistent single-underscore env convention; now matches env_var_name (__).
CI: PR-blocking Docs build (strict) job (this drift class — 295 invisible warnings, docs teaching dead APIs — can no longer merge green); zizmor pinned @1.28.0; gh-deploy --strict backstop. Green on this tree post-docs: reconcile model docstrings with griffe (Args → Attributes) #39.

Minors / coherence

TranscriptionResult.metadata removed (the spec's own no-blanket-metadata decision, applied to the result model; spec TR.1 updated) · every Diagnostic code emission uses an exported DIAG_* constant (the "single source of truth" convention was proclaimed in two modules and violated in four, including one of the proclaimers) · compliance run --bridge-timeout + transcribe/prepare --strict (+ cli.md) · honest wording in mission.md (v1 SSRF scope is advisory vs DNS rebinding) and README (interface-level compliance; no "isolated plugin" dependency claim) · spec stable_until row carries the closed-exemption caveat inline · InvalidSessionUseError docstring drops a false 422 claim · identifiers.py wrapper collapse, loader.py duplicate license header, discovery.py __main__ print block.

Directional items became issues instead of patches: #45 (capability ontology ADR — please land before #37/#8 add fields; now also tracks the wire_encodings=None transport duality), #46 (CJK segment joining), #47 (discriminated event union), #48 (result view equivalence), #52 (sync-bridge cooperative abort + subprocess isolation), #53 (first-class compliance outcomes + --runtime probe profile). #54 (WS establishment fault ownership) was opened mid-review and then fixed in this PR (closed as completed — same-domain defects found while fixing stay in scope; only the pre-agreed architectural follow-ups #52/#53 defer).

Post-review hardening (10 external review rounds; every finding adjudicated against the project's threat model, valid ones fixed in-PR)

Round 1 (9 findings): the sync-call boundary became a public runtime primitive guarding every real consumer (CLI/REST/WS) with the new EngineContractError; operator/CI logs scrub pydantic input echoes (runtime/safe_logging); server construction faults remapped by fault ownership (404/503/500 — never a caller-blaming 422); recommended_wire_format() compliance is unconditional and type-pinned; supports() is verified against the full capability tree (iter_queryable_paths + unknown-path fail-closed); doctor satisfiability is three-state (exact-oracle UNSAT only; epoch-preserving witnesses); bridge timeouts respect threading.TIMEOUT_MAX; AliasPath/mixed AliasChoices rejected at config-class definition; and the nullable-timing redesign above.
Round 2 (6 findings): fault ownership completed at transcription/WS establishment (lazy ConfigurationRequiredError → 503/service_unavailable; engine ConfigError/InvalidProviderParamError → scrubbed 500/internal_error; closes #54 in-PR); one safe exception boundary everywhere (safe_exception_summary — tainted wrapper messages withheld, streaming error-event detail scrubbed at the source, CLI --debug no longer re-opens the echo, compliance messages carry no raw repr); the CLI bridge pins AudioFormat | None; secret whitespace fidelity holds across every alias input key (one _flat_input_keys vocabulary); renderers reject unknown policies loudly and never synthesize a zero-duration whole-text cue.
Round 3 (8 findings): operator-log withholding is structural, not content-based (pydantic truncates long inputs, so substring matching against the full value is unsound — new mark_input_echo_free marker for the standard's sanitizing constructors, total safe_str so hostile __str__ can never crash a containment layer); every CLI catch arm reports through the one safe boundary (the normal no---debug error line was the leak); WS establishment maps a bare ValueError to internal_error (engine fault, never caller-blamed unsupported); secret fields resolve to exactly one carrier with defaults vetted at class definition (plaintext unions, plain-string defaults, and default_factory all fail loud; SecretBytes wraps per-carrier); from_env's explicit-wins is alias-aware; synthetic-cue visibility is decided on the output millisecond grid; streaming error-flow docs match the producer-side summarization.
Round 4 (9 findings): the redaction walk is an exception TREE (BaseExceptionGroup members included) and total under hostile __getattribute__/__setattr__/__traceback__/__repr__ (interpreter slot descriptors, last-resort type-only backstops); validator messages are safe by TYPE, not content (pydantic-core builtins + the standard_asr_ safe-type prefix; partial echoes — slices, hashes, re-encodings — are structurally redacted); the credential loc heuristic normalizes alias separators (xi-api-key); compliance's remaining raw-repr sites render through the total boundary and the bridge worker stores exception objects; the WS config handshake is a closed model (extra="forbid" — a typo'd key fails loudly, internal handshake faults are scrubbed internal_error); bytes-input credentials keep exact contents and flat input keys have exactly one owning field; cue renderability is decided on the output millisecond grid for every cue (on_missing_timestamps → on_unrenderable, sub-millisecond measured spans no longer emit invisible cues).
Round 5 (9 findings: 6 valid and fixed, 3 rejected as outside the threat model with reasons on the PR): builtin validator-message trust is a hand-audited 85-type allowlist (get_attribute_error et al. embed underlying exception text — the "builtin = fixed template" assumption was empirically false); extra_forbidden's rejected key is masked in loc when shaped like key material (field-name-shaped keys stay named — deliberate DX); the exception-tree walk is iterative, node-capped, and total to BaseException, with a fail-closed taint verdict (exception_tree_is_tainted) and a one-line, length-capped summary; a registered model whose plugin fails to load maps to a scrubbed 500/internal_error on every resolution surface (never a caller-blaming 404); the sync-bridge drive worker contains BaseException (a CancelledError no longer fakes sync_bridge_no_terminal); the secret pre-validator covers every Mapping input. Rejected: standard_asr_ prefix "spoofing" (an authored-content trust contract, same class as mark_input_echo_free/diagnostics — plugins are operator-installed in-process code), unconditional loc placeholders (would destroy the deliberate typo-naming DX; a structural shape rule covers the material case), and the "security blocker" framing of the walker work (clients cannot construct exception groups or deep chains over the wire — fixed as robustness).
Round 6 (6 findings, all valid and fixed — the reviewer accepted the round-5 threat-model adjudication, so this round was about the narrow designs meeting their own rules): an audited builtin authorizes a message only when the message IS its canonical rendering (the error type is caller-settable, so PydanticCustomError("int_parsing", …) inherited the authorization — the rendering is now recomputed from the entry's ctx and compared, with numeric ctx holes type-checked); every caller-authored loc component is filtered, not just extra_forbidden's (a dict[str, T] field contributes the caller's mapping key — the round-5 "only channel" claim in IC.3/§1 was false and is retracted), with an audience="caller"|"operator" split so operator sinks drop rejected keys the caller surface still names; the exception-tree walk bounds WORK rather than rendered nodes (a 100k-member group no longer costs 100k entries to render 100) and a fail-closed verdict from an incomplete scan says "may carry" instead of asserting an unseen echo; the supports() classification probe contains BaseException (it sits inside an except block, where a sibling arm never catches); the server spec's four self-contradictions about the 404/500 split are resolved against §3.7; and the app-developer error guide stops teaching .detail/.engine_id, which never existed.
Round 7 (3 findings, all valid and fixed — two were places where the round-6 PROOF was weaker than its docstring claimed): builtin message admission now requires a hole-free or numeric-only template and pins the audited template string (14 text-context types such as string_pattern_mismatch rendered a hand-raised PydanticKnownError's ctx canonically, and recomputing from the installed template could never detect drift — the round-6 fail-closed claim was wrong); operator renderings print a loc only when the validated model PROVES it is schema-derived (model_paths_are_static), since a short identifier-shaped dict[str, T] mapping key is indistinguishable from a field name by shape; and a raw ValidationError at the CLI's top level is an engine fault (exit 1, operator audience), not a usage error — every caller-originating pydantic failure is already classified upstream, and the old exit 2 also split the trust boundary between the normal line and the --debug trace.

Round 8 (3 findings, all valid in substance and fixed — two closed with the reviewer's preferred provenance design rather than the fallback): builtin message admission is now proved at two levels — an entry alone admits only hole-free audited templates, while templates with holes (numeric included: a hand-raised PydanticKnownError can carry a cross-field input value, a numeric credential, or an integer re-encoding — the round-7 "a number cannot carry text" claim is retracted, and IC.3 no longer contradicts its own no-re-encoding rule) need the new closed-schema proof, under which the 14 audited text-hole types return to proven surfaces; model_paths_are_static is replaced by model_schema_is_closed, a walk of the ACTUAL pydantic core schema (annotation walking was unsound: Any+TypeAdapter revalidation, TypedDict/dataclass mapping members, from_exception_data injection, caller keys spelled "[key]", integer mapping keys — unproven operator renderings now mask every non-prefix loc component, integers and marker shapes included); and CLI exit codes classify fault at the seam — FactoryLoadError and the ValueError family escaping the engine execution seam exit 1, engine DECLARATION defects (malformed declared language tags, IC.6 violations, unsatisfiable prepare shapes) raise the widened EngineContractError, and every ConfigError stays the invoker-actionable exit 2 because at the CLI the invoker owns both the flags and the env (ownership follows the config supplier, not the exception site).

Round 9 (4 findings, all valid and fixed — one root cause, two channels, plus two CLI consumers the round-8 contract did not reach): the closed-schema proof audited node KINDS and function-* nodes, so third-party code running from an ordinary-looking node was invisible to it — a default_factory deriving a bound from another input field (pydantic ≥2.10 hands it the data) put a numeric PIN into a canonical greater_than message, a dataclass __post_init__ running an inner TypeAdapter put a caller mapping key into loc and a caller-derived pattern into string_pattern_mismatch, and model_post_init / custom_init / is-instance / is-subclass / enum._missing_ hooks did the same. The root cause is the ENUMERATION (listing sub-schema slots or per-kind hook fields is fail-open and gains a hole with every pydantic release), so the sweep is now total over the schema minus the non-validation subtrees, requiring any reachable callable to be audited machinery — metaclass included, since __instancecheck__ / __subclasscheck__ / _missing_ live there — and RESOLVING hook flags to their method rather than rejecting them (pydantic points post_init at its own init_private_attributes on private-attribute models, and BaseProperties/DeclaredCapabilities/every SecretStr config depend on default_factory and is-instance, so a blanket rejection would have deleted the operator DX round 8 restored). On the CLI, show was the one consumer that caught FactoryLoadError, printed the sanitized unavailable line, and returned 0 — it now keeps the diagnostics and exits 1 — and the engine-fault seam was moved to start at the ATTRIBUTE LOOKUP, since prepare and declared_capabilities may be descriptors whose bodies are plugin code.

Round 10 (12 findings: 10 valid and fixed, 1 declined with cross-verification, 1 declined as outside the threat model): the closed proof stopped at the SCHEMA, so the class's own construction lifecycle ran author code it never saw — a PrivateAttr factory, a mutable default's __deepcopy__, and a custom __new__ (which receives the caller's keyword data on the from_env construction path). Enumerating lifecycle hooks would have been fail-open exactly as enumerating schema hooks was, so the sweep is now total over the DISPATCH SURFACE — the dunder protocol, the only route by which the interpreter and pydantic reach author code — across both MROs, plus private-attribute defaults and the type of every value the schema carries (which also recurses the proof into nested models). Separately, raise ... from None was laundering a copied input echo: the taint walk followed Python's DISPLAY graph, so a plugin wrapper that interpolated a ValidationError and then suppressed it read as clean, and the credential reached the log record, compliance issue messages, and streaming error details verbatim — the walk now follows PROVENANCE and labels the suppressed edge. The scrubbed logging path also broke logging's own % semantics (appending the safe tree as an argument turned a literal % and a mapping argument into handler errors, dropping the record entirely), the sync boundary let a suspended coroutine's finally displace its verdict, the two EngineBase author hooks were dereferenced before any boundary saw them, the config input-key vocabulary counted serialization-only aliases as input keys, the WS diagnostics delta could not see the in-place overflow summary (a two-layer drift against G.5.2), and an empty segment raised SubtitleRenderingError where both renderers would have skipped it. Declined: bounding the batch-only compliance probe (main already calls the same methods unbounded in the default run, so it is no new risk class — it belongs to #53's runtime-probe execution model), and hardening traceback/type metadata against crafted code objects (an adversarial in-process plugin, outside the threat model).

Round 11 (3 findings, all valid and fixed): the closed proof walked the copied default's TYPE, not its VALUE graph — pydantic deep-copies field defaults per instance (dict keys included), so a custom __deepcopy__ on a dict key / deque element / opaque holder ran unseen; values now pass a value-graph closure rule (exact copy-atomic scalars, profile-locked audited scalars, exact containers with closed contents, lifecycle-closed BaseModel/enum instances — everything else open, fail-closed). The taint walk followed only the DISPLAYED chain edge, so raise Wrap(f"…{validation_error}") from RuntimeError("benign") — explicit cause taking display precedence over the tainted __context__ — read as clean; both edges are now traversed. And the sync boundary's classification is total (verdict kinds, contained naming).

Round 12 (6 findings, all valid and fixed — a rare full-validity round; two fixed with a different design than the reviewer prescribed): (1) BaseException.args was a taint-walk bypass — str(RuntimeError(validation_error)) IS the input echo, with no chain edge and no hostile code — args joins the provenance walk as a bounded, cycle-safe VALUE-GRAPH edge (exact builtin containers recurse, dict keys included; unrecognized shapes fail the verdict closed without asserting an echo; found exceptions become held in args nodes contributing their own edges). Implementing this exposed a second-order hole the review missed: a MARKED input-echo-free wrapper holding the error in its own args rendered the echo through the very branch that trusts marks — the mark is now honored structurally (exception references or unvettable values in a node's own args disprove it). (2) supersede silently reordered untimestamped transcripts (final(a) final(b) supersede([a],[a2]) final(a2) → "world hi"), and the spec's own dict-shaped core reduce couldn't express placement — spec §5.2 now pins the contiguous-live-block + in-place-splice invariant (suppression + supersede_noncontiguous_old_ids on violation, same stance as the cross-speaker ban), with a shared reading-order ledger behind the guard (compliance replay inherits it) and the reducer, reduce_event reshaped to the spec snippet's (order, texts) state, and golden wire-shaped traces driving both reduces to identical results. (3) public_dump() ran author serialization hooks inside the "masked" dump (@computed_field authorization, @model_serializer shadow keys, @field_serializer reading a sibling secret) — fixed by CLOSING the serialization surface at class definition on every BaseConfig (rejecting all author hooks, declared/inherited/Annotated forms) rather than the reviewer's parallel per-field re-serializer: the dump engine stays pydantic's own, and the closure also restores the dump-is-the-declared-input-surface round-trip. The carrier/marker split closes with it (an unmarked SecretStr silently strips credential whitespace — carrier now requires secret_field, top-level and nested). (4) Windows py3.13 CI was red (ModuleNotFoundError: tzdata under a real ZoneInfo audit fixture) — tzdata joins the test dependency group only. (5) an empty reduced result said segments=null where the spec's null rule requires [] (performed-but-empty; the renderers' zero-cues-vs-fallback-cue split rides on it). (6) the "one line" summary escaped only C0 — U+0085/U+2028/U+2029 forged records in line-oriented logs; the full str.splitlines boundary set is escaped and every summary exit (fallback placeholders whose type name is caller-controlled text included) canonicalizes through the same escape.

Round 13 (2 findings, both valid and fixed — the reviewer re-verified rounds 11-12 and confirmed 5/6 closed, then found F3's closure was still an ENUMERATION): public_dump()'s "closed serialization surface" rejected the three serializer decorators and the two Annotated serializer forms, but SerializeAsAny[T] walked straight through — it serializes the runtime object, so a subclass adding a SecretStr plus a @computed_field rematerialized Bearer sk-... inside the masked dump, with no decorator on the config (the registry saw nothing) and no credential in the declared type (the nested-carrier scan saw nothing). Confirmed on this head before fixing.

The fix is the route the reviewer recommended as the long-term one, not the minimal denylist patch, because verifying their finding surfaced three more channels of the same class that a denylist would keep missing: a nested submodel's own hooks run inside the parent's dump (its computed field is emitted as a key of the nested object); a custom __get_pydantic_core_schema__ installs an arbitrary serializer function on an ordinary-looking field, with no decorator and no annotation metadata anywhere; and an undeclared value shape (Any / object / an unparametrized container / dict[str, Any]) is the same duck-typing reached from the declared TYPE instead of a marker — pydantic hands whatever object the field holds at runtime to that object's own serializer, and the schema carries no serialization node at all to audit. So the closure is now a total sweep of the schema's serialization surface (redaction.serialization_closure_gap), the exact mirror of model_validation_is_closed: that proof excludes serialization/computed_fields by a timing argument, and this one takes precisely them as its object, so the two together cover pydantic's whole author-code dispatch surface for one model. Rules: non-empty computed_fields at any depth are refused structurally (they name code by property name — there is no callable in the node to audit); every serialization node must carry an audited kind, no duck-typing marker, and only pydantic-authored callables; every reachable any validation node is a gap; Field(exclude=True) is refused for the round-trip half of the same contract (a dump documented for persistence must not silently drop a declared input); unknown kinds and introspection failures are gaps. Two subtleties the tests pin: the duck-typing marker has two shapes across the supported range — a bare {"type": "any"} on pydantic 2.13, but pydantic's OWN function-wrap lambda (trusted provenance, audited kind) wrapping it on the 2.5 floor, so auditing only the outer node passes the floor — and a return_schema stays under the callable-only rule, since it describes a return TYPE and a -> dict[str, Any] annotation legitimately contains an any node. The three decorator checks remain as a fast, precisely-worded front end; the proof is the backstop. Refusing Any also restores what G1.3/G3.1 already promised (an Any field's JSON Schema is empty, so it renders as nothing in the config form), and a heterogeneous mapping is spelled dict[str, str | int | bool | None] or a named submodel.

Verifying the finding also surfaced the same contract broken from the KEY side: a subclass setting model_config extra="allow" stores undeclared caller input on the instance and public_dump emits it verbatim, past a by-name mask that knows only declared fields, while extra="ignore" silently swallows a mistyped credential key so it reads as an absent credential (a compliance SKIP) instead of a loud error. extra="forbid" was never stylistic here — the flat input-key vocabulary, the absent-vs-invalid classifier, and the typo-names-the-key DX all assume every accepted key belongs to a declared field — so a config reopening it is refused at definition, and a nested submodel with extra="allow" (which carries undeclared keys into the parent's dump just the same) is refused by the schema proof.

Separately in this round: the construction-lifecycle proof was fail-open on the descriptor DISPATCH object — it unwrapped the first of __func__/fget and vetted only that, so a property with a trusted getter plus a plugin fset, and a staticmethod/classmethod subclass overriding its own __get__, both proved a model closed (pydantic dispatches __pydantic_fields_set__ while an instance is built, giving the setter the same nested-TypeAdapter graft channel). The check is now total over what executes: the descriptor's TYPE, a property's fget/fset/fdel, a static/classmethod's __func__, each recursing through the same rule.

Round 14 (2 findings: one valid and fixed, one adjudicated as a contract overclaim — fixed by stating the boundary, with the reviewer's prescribed mechanism rejected for cause):

P2 (valid, fixed): the input surface was closed only at the root. The round-13 rule refused extra != "forbid" on the config and extra="allow" on nested submodels, but a nested submodel left on pydantic's DEFAULT (extra="ignore" — and the same silent-drop default for TypedDict and dataclasses) accepted a typo'd nested key and dropped it with no diagnostic: {"decode": {"baem": 8}} read as applied while the engine ran on the field's default, and a credential key misplaced into a submodel vanished the same way. A config class is now refused at definition unless every nested input container its core schema reaches (submodel / TypedDict / pydantic dataclass, through containers, unions, and depth alike) forbids undeclared keys. The guard reads the EFFECTIVE policy from the schema artifact (config.extra_fields_behavior — same key on the 2.5 floor and current pydantic, verified on both), so pydantic's config-propagation rules are honored rather than re-derived: a bare TypedDict and a bare stdlib dataclass inherit the config's forbid and stay closed for free (pinned by test on both versions), while a pydantic dataclass owns its config and must close itself. A policy the walk cannot read as forbid — including a future pydantic that stops emitting the key — fails closed at class definition. Runs after the schema proof, so extra="allow" keeps its precise dump-channel message.

P1 (adjudicated: real counterexample, wrong prescription — the defect was the contract's overclaim). The reviewer's LeakyPath reproduces on this head, floor and current alike: pydantic's own audited ser_path calls str() on the runtime value, so a validator-returned Path SUBCLASS whose __str__ embeds get_secret_value() emits Bearer sk-... through public_dump(). But the counterexample's leak edge is not the serializer — it is the author act of copying the secret out of its carrier into non-secret state, and that act defeats ANY serialization mechanism: the equivalent one-liner writes get_secret_value() into a plain declared str field, where the leaking value passes every type and dispatch audit (a plain str whose CONTENT nothing can classify as secret — verified side by side). The prescribed serialization_value_closure_gap(instance) therefore does not close the leak class it targets: it forces the same copy through a one-line-different costume, while costing a full value-graph walk on every dump (/v1/models hot path), failing at RUNTIME on previously-working configs instead of loudly at class definition, and requiring exact-type audits that legitimate values (enum members are instances of author classes; Path() returns platform subclasses) break by construction. What WAS wrong is the contract's wording: "nothing author-defined runs inside the dump" claimed more than a schema proof can hold. The contract now states the boundary precisely — the proof bounds what the SCHEMA installs in the dump, not the contents of values author code constructed; the value-level envelope is and remains the carrier contract (IC.3) — in public_dump's docstring, the sweep's docstring, spec §776, and the engine-author guide, and the boundary is pinned executable (test_secret_extraction_is_the_closure_boundary, both costumes, including the frozen config making the naive field write loud while value mutation sidesteps it). This is the same resolution shape as F1's str()-flattened-exception boundary: state the limit honestly rather than build machinery that cannot hold it.

Round 15 (5 findings, all valid and fixed — the reviewer could not clone the repo, so every counterexample was re-run against the real code before acting; all five reproduced, and verifying one of them surfaced a regression of my own):

F1 (Critical): an author-defined __str__ walked straight past the taint proof. The provenance walk answers what a tree HOLDS — chain edges, group members, the args value graph — and an ordinary SDK wrapper shape defeats all three at once: keep the ValidationError on an attribute, and args hold one harmless string, both chain edges are None, no group member exists. A complete, untruncated scan proved the tree clean while str() rendered the full input echo into safe_exception_summary and, through the untainted delegation to Logger.exception, into the operator log verbatim. The fix proves the other half — what CODE produces the text: each node's __str__/__repr__/__format__ is resolved the way the interpreter resolves it (on the TYPE, through the MRO) and must come from audited machinery, else the message is withheld and the tree never reaches the native formatter. Auditing the STATE instead was rejected for cause: enumerating where an error can hide is the fail-open pattern this module was burned by twice, and it cannot close the case at all — a display that revalidates held raw input BUILDS its echo at render time and holds nothing to find. The deliberately-kept operator channel survives, because the audit is narrow by construction: an exception that does not override display — the overwhelmingly common shape, including nearly every SDK's error type — inherits BaseException.__str__ (provenance builtins) and keeps its message, and the verdict is per NODE so an audited sibling in a clean chain keeps its message too. Hostile-display containment gets stronger: such a display is now never dispatched at all, where the old placeholder path called it and merely survived the raise.

F2 (High): the env codec served fewer shapes than the guards admit — plus a regression of mine. Mapping[str, int], Sequence[str], a TypedDict and a stdlib dataclass all pass every class-definition guard yet had no annotation-ORIGIN entry, so each was unreachable through its own documented STANDARD_ASR_<ENGINE>__<FIELD> convention. The classification now walks the field's core schema and stops at the first structured kind. The bias is deliberate and asymmetric: guessing "raw" for something structured fails LOUDLY at construction, while guessing "json" for a scalar silently reinterprets it ("123" → the integer 123), so "json" needs positive evidence and anything unrecognized stays raw. A field reaching BOTH (str | list[str]) has no defined reading — either choice disagrees with the explicit constructor, which always takes the string — and is refused at class definition. Verifying this also exposed a round-13 regression of my own: the audited serialization kinds rejected a plain Sequence[str] field on current pydantic (its ser chain wraps an inner {"type": "str"}) while passing on the 2.5 floor, which does not emit that node — a version-shaped false positive on a legal config field, now fixed.

F3 (High): any factory exception aborted the whole compliance run. registry.create wraps only a construction-time ValidationError, so a factory's own RuntimeError/TypeError/OSError escaped the named-types catch and left for name in names — every LATER model produced no report at all, which is precisely the one-command guarantee (G2.1) the command exists to provide. check_entrypoints's earlier isolated call cannot compensate: _run_instance_checks constructs a second, independent time. The arm now catches Exception, with a second per-model envelope so even a crashing check implementation stays one model's verdict under its own compliance_check_crashed code. BaseException is deliberately not caught, and a test pins that KeyboardInterrupt still stops the run.

F4 (High): the metadata endpoints' boundary stopped at class resolution. Everything after it is third-party code — the descriptor read (a metaclass property is a legitimate shape), canonical_json(), model_json_schema() with its custom hooks — and a raise anywhere in that stretch left through Starlette's unhandled path: an undocumented plain 500, and log_exception_safely never ran, so the ASGI server's native traceback logger rendered the chain. Verified: a ValidationError from a metaclass property printed its input echo into the operator log. One shared helper now spans resolution → descriptor read → projection, passing the endpoints' own deliberate verdicts through and mapping everything else to a safe-logged scrubbed 500. These are unauthenticated discovery surfaces, so "a plugin descriptor does not usually fail" was never a boundary.

F5 (High): the public models accepted values they could never send. Declared Any, Diagnostic.provided/effective and every extra mapping admitted Python objects with no JSON representation while the server spec promised to forward them VERBATIM — two promises that cannot both hold, and the hidden "JSON-safe Any" precondition appeared in no type, validator, or document. The slots are now JsonValue (non-finite floats excluded: NaN/Infinity are Python floats but not JSON), so the failure lands at construction, naming the field. Three concrete failures close with it, each verified first: an error event dropped extra AFTER serializing, so an unserializable extra cost the client every safe structured field the protocol documents (code, recoverable, retriable_after, the gap/reconnect fields) — drop-before-send is an order, not an outcome; the WS initial diagnostics frame was built between two boundaries and killed the route unhandled; and the REST projection ran in the ASGI encoder, after the endpoint returned and past every fault arm. to_json_value is the one helper for handing a structured value or a typed container to such a slot (the list-invariance complaint is a static artifact, absorbed once instead of a cast per site).

Gates

At head ce680d9: ruff format/check clean · pyright 0 errors · pytest 2251 passed, 100% branch coverage · mkdocs build --strict clean · uv lock --check clean · package builds. The pydantic lower-bounds lane (2.5 floor, py3.10) was additionally run locally for this round: 2243 passed — it is where F2's version-shaped false positive and F1's floor-vs-current schema shapes are actually discriminated.

Merge-order notes for the other open PRs

This PR first (fully verified tree; docs gate green post-docs: reconcile model docstrings with griffe (Args → Attributes) #39).
chore: migrate tests to starlette 1.x TestClient; lift starlette<1.0 constraint #40 next — on rebase, its httpx→httpx2 migration should also convert the 3 httpx.Response annotations in this PR's new tests/test_server.py tests (mechanical).
ci: bump the github-actions group across 1 directory with 4 updates #43 — independent, any time.
fix: make the toolchain honor its own promises #41 — one real conflict in cli.py _run_sync_bridge: keep check_sync_bridge(_factory, timeout=timeout) while taking fix: make the toolchain honor its own promises #41's recommended_wire_format() inlining; cli.md sections compose.
docs: roadmap consolidation, staleness refresh, settled design cases #38 last (it depends on fix: make the toolchain honor its own promises #41's CLI transcript anyway). ⚠️ Its streaming.md event-table hunk predates this PR's fixes — resolve toward main, or the error-row semantics fix is silently reverted.

Summary by CodeRabbit

New Features
- Added stricter protocol, compliance, capability, wire-format, and synchronous-operation checks.
- Added configurable subtitle handling for unrenderable or untimed segments.
- Improved streaming ordering, segment replacement, timestamps, diagnostics, and JSON-safe output.
- Added clearer errors for missing configuration, engine contract failures, and subtitle rendering issues.
Bug Fixes
- Improved credential redaction and fault classification across CLI, servers, logs, and diagnostics.
- Refined dependency analysis and conflict reporting.
Documentation
- Expanded guidance for streaming, security, configuration, discovery, compliance, and server behavior.
- Added strict documentation builds to continuous integration.

github-actions · 2026-07-26T11:14:54Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details
actions/astral-sh/setup-uv	fac544c07dec837d0ccb6301d7b5580bf5edae39	Unknown	Unknown

Scanned Files

.github/workflows/mkdocs.yml

coderabbitai · 2026-07-26T11:15:02Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 978b9f51-98d5-4321-9e8e-99f5fc8b6401

📥 Commits

Reviewing files that changed from the base of the PR and between f976506 and 8f94403.

📒 Files selected for processing (2)

CHANGELOG.md
docs/spec/specification.md

💤 Files with no reviewable changes (1)

CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (1)

docs/spec/specification.md

📝 Walkthrough

Walkthrough

The pull request updates protocol contracts, JSON serialization, streaming reduction, subtitle rendering, configuration security, error redaction, compliance checks, CLI and server fault handling, dependency analysis, documentation, tests, and CI workflows.

Changes

Standard ASR contracts

Layer / File(s)	Summary
Contract and wire-value foundations `src/standard_asr/contract/*`, `src/standard_asr/runtime/interface.py`, `src/standard_asr/audio/conversion.py`	The protocol adds synchronous hook and wire-format requirements. JSON wire values, nullable timestamps, language errors, capabilities, and diagnostic constants use stricter contracts.
Synchronous protocol boundary `src/standard_asr/runtime/protocol_boundary.py`, `src/standard_asr/contract/exceptions.py`	Synchronous result validation classifies awaitables, wrong types, and unclassifiable values. Engine contract errors avoid exposing returned values.
Streaming and rendering `src/standard_asr/runtime/streaming.py`, `src/standard_asr/renderers.py`	Streaming reduction preserves reading order and in-place supersede placement. Subtitle rendering supports `error`, `omit`, and `collapse` policies for unrenderable spans.
Configuration and fault boundaries `src/standard_asr/runtime/config.py`, `src/standard_asr/runtime/redaction.py`, `src/standard_asr/toolchain/*`	Configuration inputs and secret carriers are restricted. CLI, server, discovery, and compliance paths classify faults and sanitize validation details.
Documentation and CI validation `.github/workflows/`, `docs/`, `README.md`, `CHANGELOG.md`, `tests/*`	Documentation builds use locked dependencies and strict mode. CI pins the SARIF tool and requires the documentation job. Specifications, guides, and regression tests describe and validate the revised behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Plugin catalog: pre-install discovery for the ecosystem #28 — The compliance, discovery, and metadata changes support the planned plugin-catalog verification pipeline.

Possibly related PRs

standard-voice/standard_asr#5 — Both pull requests modify the CI and MkDocs workflows.
standard-voice/standard_asr#31 — Both pull requests modify diarization-related rendering, streaming, compliance, and protocol surfaces.
standard-voice/standard_asr#32 — Both pull requests modify the reorganized contract, runtime, compliance, and streaming modules.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.81% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the review sweep and its primary outcomes: documentation drift, strict CI gating, and doctor dependency-floor analysis.
Description check	✅ Passed	The description thoroughly explains the changes, related issues, review scope, validation results, and merge-order notes, despite not reproducing the template headings.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/design-review-sweep

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-07-26T11:15:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

+            A wire format the engine's session-establishment guard accepts, or
+            ``None`` when no bare-frame streaming format can be recommended.
+        """
+        ...


coderabbitai

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/for_app_dev/streaming.md (1)
12-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document the None result path.

recommended_wire_format() may return None when no usable bare-frame rate exists, but this example and its prose treat it as always returning AudioFormat. Show the fallback/error path before passing it to start_transcription().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/for_app_dev/streaming.md` around lines 12 - 20, Update the streaming
example around recommended_wire_format() to handle a None result before passing
audio_format to start_transcription(). Show an explicit fallback or error path
for the unavailable format, while preserving the existing custom AudioFormat
guidance.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/mkdocs.yml:
- Around line 50-53: Update the deployment workflow surrounding the `mkdocs
gh-deploy --force --strict` step to use the same Python 3.13 environment and
locked dependencies as the documentation job in `.github/workflows/ci.yml`,
including the equivalent `uv sync --locked` setup. Ensure MkDocs runs from that
synchronized environment rather than floating package installations.

In `@docs/for_app_dev/errors.md`:
- Line 23: Add an InvalidSessionUseError row to the exception table in
errors.md, documenting triggers for mixing managed and manual input, calling
feed() repeatedly, and iterating the event stream repeatedly, along with
appropriate recovery guidance.

In `@README.md`:
- Around line 97-101: Update the process-isolation statement near the
dependency-conflict description to clarify that isolation can resolve
plugin-versus-plugin conflicts but not plugin-versus-core conflicts, since each
isolated worker still imports the core and must satisfy its NumPy requirement.

In `@src/standard_asr/contract/language.py`:
- Around line 240-242: Update the `mode` parameter description in the relevant
docstring to explicitly include its documented type, `str | None`, while
preserving the existing explanation of its purpose and strict-mode error
behavior.

In `@src/standard_asr/runtime/interface.py`:
- Around line 182-200: Update EngineBase.recommended_wire_format to return None
whenever the engine’s Properties.streaming_input is false, before deriving a
format; retain the existing derivation for streaming-capable engines. Add or
update coverage for an output-only engine to verify it receives None.

In `@src/standard_asr/toolchain/cli.py`:
- Line 721: Update the docstring for the function containing the
`bridge_timeout` parameter to include an `Args:` entry identifying it as a float
and describing that it specifies the timeout for the whole session, while
preserving documentation for the other parameters.
- Around line 204-216: Update the --bridge-timeout argument in run_parser to
validate that the parsed value is finite and strictly greater than zero,
rejecting zero, negative, and non-finite inputs as argparse usage errors while
preserving the 5.0 default. Add parser tests covering each rejected value.

In `@tests/test_language.py`:
- Around line 24-36: Add concise English Google-style docstrings to each public
test function at tests/test_language.py ranges 24-36, 247-274, 277-294, 335-355,
and 394-433, describing diagnostic literal contracts, strict candidate
rejection, mode propagation, over-limit rejection, and invalid-candidate
behavior respectively. Add corresponding docstrings at tests/test_streaming.py
ranges 2934-2946, 3080-3102, and 3105-3162 for unknown deadline-field rejection,
streaming diagnostic literals, and timestamp reducer behavior. Preserve all
existing test logic and assertions.

---

Outside diff comments:
In `@docs/for_app_dev/streaming.md`:
- Around line 12-20: Update the streaming example around
recommended_wire_format() to handle a None result before passing audio_format to
start_transcription(). Show an explicit fallback or error path for the
unavailable format, while preserving the existing custom AudioFormat guidance.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 340424f3-044f-42af-b77a-3a3b5b72be28

📥 Commits

Reviewing files that changed from the base of the PR and between fb13e8b and 4253c3d.

📒 Files selected for processing (33)

.github/workflows/ci.yml
.github/workflows/mkdocs.yml
README.md
docs/for_app_dev/errors.md
docs/for_app_dev/streaming.md
docs/mission.md
docs/spec/cli.md
docs/spec/specification.md
src/standard_asr/audio/conversion.py
src/standard_asr/audio/loader.py
src/standard_asr/compliance.py
src/standard_asr/contract/exceptions.py
src/standard_asr/contract/identifiers.py
src/standard_asr/contract/language.py
src/standard_asr/contract/properties.py
src/standard_asr/contract/results.py
src/standard_asr/plugins/discovery.py
src/standard_asr/renderers.py
src/standard_asr/runtime/gating.py
src/standard_asr/runtime/interface.py
src/standard_asr/runtime/streaming.py
src/standard_asr/toolchain/cli.py
src/standard_asr/toolchain/doctor.py
tests/test_asr_interface.py
tests/test_audio_conversion.py
tests/test_cli.py
tests/test_compliance.py
tests/test_doctor.py
tests/test_language.py
tests/test_param_gating.py
tests/test_results.py
tests/test_server.py
tests/test_streaming.py

💤 Files with no reviewable changes (2)

src/standard_asr/plugins/discovery.py
src/standard_asr/audio/loader.py

- mkdocs deploy now builds from the SAME locked uv environment (Python 3.13, uv.lock, all extras) as the CI docs gate: a green PR gate now guarantees an identically-rendered, publishable deploy (previously floating pip installs on Python 3.x could drift). - compliance run --include-bridge skips output-only engines with a loud note instead of spuriously failing: the bridge feeds bare PCM frames, which fail the streaming_input capability gate before any bridging -- a failure about the check's shape, not the engine. recommended_wire_format() stays deliberately capability-blind (Properties-only, per spec §3.1); both docstrings now state that division of labor, and streaming.md documents the None return and the supports("streaming_input") pre-check. - --bridge-timeout validates finite > 0 at parse time (0/negative/inf/nan became an instant false "did not terminate" or a hang blamed on the engine). - errors.md gains the missing InvalidSessionUseError table row; README qualifies process isolation (it fixes plugin-vs-plugin, not plugin-vs-core, matching doctor's own attribution); language.py documents mode's type; _run_instance_checks documents bridge_timeout; docstrings added to the review-flagged test functions. Not changed, with reasons recorded on the PR: the protocol stub `...` is the file-wide Protocol idiom (static-analysis false positive), and recommended_wire_format() keeping its Properties-pure contract is the spec'd design -- the real harm (CLI bridge on output-only engines) is fixed at the call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

t41372 · 2026-07-26T11:30:19Z

Addressed the CodeRabbit review in 763cd87. Dispositions:

Fixed (7):

mkdocs.yml deploy env drift (Major) — deploy now uses the same locked uv environment as the CI docs gate (Python 3.13, uv sync --locked --all-extras --all-groups, pinned setup-uv), so a green PR gate guarantees an identically-built deploy.
--bridge-timeout validation — new _positive_finite_seconds argparse type rejects 0/negative/inf/nan as usage errors (they turned into an instant false "did not terminate" or a hang blamed on the engine); parametrized tests added.
errors.md missing InvalidSessionUseError row — added with triggers (mixing feed/manual, double feed(), double iteration) and the "session is NOT closed — fix the calling code" guidance.
README process-isolation claim — qualified: isolation is the escape hatch for plugin-vs-plugin conflicts; a plugin incompatible with the core's own numpy floor cannot run anywhere, and doctor reports that as its own conflict (matches doctor's new attribution model exactly).
mode docstring type (language.py) and bridge_timeout Args entry (_run_instance_checks) — documented.
Test docstrings — added to all the flagged functions in test_language.py / test_streaming.py.
Outside-diff: streaming.md None path — documented, plus the supports("streaming_input") pre-check guidance.

The recommended_wire_format() finding — harm fixed at the right layer, contract deliberately kept: the real failure you identified (CLI --include-bridge driving start_transcription(audio_format=...) on an output-only engine) is fixed in the CLI: the bridge now runs only for streaming_input engines and prints a loud skip note otherwise, with a test proving check_sync_bridge is never invoked (the failure was about the check's shape, not the engine). The method itself stays capability-blind by design — spec §3.1 pins the derivation as a pure, class-level Properties fact, and the compliance round-trip (format ⊆ ensure_stream_format_supported) must hold for every engine; whether a bare-frame session can open is the streaming_input gate's job inside start_transcription. Both docstrings and the streaming guide now state that division of labor explicitly.

Skipped (1): github-code-quality's "statement has no effect" on the protocol stub — the ... body is the standard typing.Protocol member idiom used by every other method in the same class (transcribe, supports, …); not a defect.

Gates after the fixes: ruff format/check clean · pyright 0 errors · pytest 1770 passed, 100% coverage · mkdocs build --strict clean.

🤖 Generated with Claude Code

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

tests/test_compliance.py (1)

1002-1007: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate _HangSession test double.

This redefines the same _HangSession class already used locally in test_sync_bridge_deadlock_reports_timeout (Lines 904-911). The file already hoists other shared session doubles to module scope (e.g. _NoTerminalSession, reused at Line 1014) -- consider doing the same here to avoid two copies drifting apart.

♻️ Proposed refactor

+class _HangSession(TranscriptionSession):
+    async def _open(self) -> None:
+        time.sleep(1.0)
+
+    async def _produce(self) -> AsyncIterator[TranscriptionEvent]:
+        yield TranscriptionEvent.done()  # pragma: no cover - never reached
+
+
 def test_sync_bridge_deadlock_reports_timeout(monkeypatch: pytest.MonkeyPatch) -> None:
-    class _HangSession(TranscriptionSession):
-        async def _open(self) -> None:
-            time.sleep(1.0)
-
-        async def _produce(self) -> AsyncIterator[TranscriptionEvent]:
-            yield TranscriptionEvent.done()  # pragma: no cover - never reached
-
     report = check_sync_bridge(_HangSession, timeout=0.05)
     ...

And in test_sync_bridge_issues_carry_the_model_key, drop the local redefinition and rely on the module-level class.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_compliance.py` around lines 1002 - 1007, Hoist the duplicate
_HangSession definition from test_sync_bridge_issues_carry_the_model_key to
module scope alongside the existing shared session doubles, retaining its
current _open and _produce behavior. Remove the local class definition from that
test and reuse the module-level _HangSession, while leaving
test_sync_bridge_deadlock_reports_timeout behavior unchanged.

src/standard_asr/toolchain/cli.py (1)

90-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the repeated --strict-discovery help text into a shared constant.

The same 5-line help string is duplicated verbatim across six add_argument call sites. A future wording tweak (this text specifically documents a past --strict vs --strict-discovery naming fix) risks updating some copies and missing others, producing inconsistent --help output.

♻️ Proposed refactor

+_STRICT_DISCOVERY_HELP = (
+    "Fail on invalid plugin entry points during discovery. (Named "
+    "--strict-discovery, not --strict: 'strict' alone is the engine's "
+    "strict/best_effort PARAMETER-gating policy, an init-config field "
+    "set via --set strict=... -- a different knob.)"
+)
+
 def _add_inspection_subcommands(subparsers: Any) -> None:
     ...
     list_parser.add_argument(
         "--strict-discovery",
         action="store_true",
-        help=(
-            "Fail on invalid plugin entry points during discovery. (Named "
-            "--strict-discovery, not --strict: 'strict' alone is the engine's "
-            "strict/best_effort PARAMETER-gating policy, an init-config field "
-            "set via --set strict=... -- a different knob.)"
-        ),
+        help=_STRICT_DISCOVERY_HELP,
     )

Apply the same substitution at the other five call sites (show_parser, prepare_parser, ep_parser, run_parser, transcribe's parser).

Also applies to: 110-124, 140-154, 208-222, 246-255, 312-321

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/standard_asr/toolchain/cli.py` around lines 90 - 99, Extract the
duplicated --strict-discovery help text into one shared module-level constant,
then replace the inline help strings in the list, show, prepare, ep, run, and
transcribe parser add_argument calls with that constant. Preserve the wording
and existing parser behavior exactly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/for_asr_dev/plugin_entrypoints.md`:
- Line 37: Update the compliance command description in plugin_entrypoints.md to
state that standard-asr compliance entrypoints --strict-discovery converts
discovery failures into an entrypoint_invalid error and exits non-zero, rather
than raising EntrypointValidationError to the caller. Preserve the
discover_models(strict=True) behavior statement.

---

Nitpick comments:
In `@src/standard_asr/toolchain/cli.py`:
- Around line 90-99: Extract the duplicated --strict-discovery help text into
one shared module-level constant, then replace the inline help strings in the
list, show, prepare, ep, run, and transcribe parser add_argument calls with that
constant. Preserve the wording and existing parser behavior exactly.

In `@tests/test_compliance.py`:
- Around line 1002-1007: Hoist the duplicate _HangSession definition from
test_sync_bridge_issues_carry_the_model_key to module scope alongside the
existing shared session doubles, retaining its current _open and _produce
behavior. Remove the local class definition from that test and reuse the
module-level _HangSession, while leaving
test_sync_bridge_deadlock_reports_timeout behavior unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 104fdde4-b227-45d1-80d9-b72e9608a3a1

📥 Commits

Reviewing files that changed from the base of the PR and between 4253c3d and 5732f0c.

📒 Files selected for processing (22)

.github/workflows/mkdocs.yml
README.md
docs/for_app_dev/errors.md
docs/for_app_dev/streaming.md
docs/for_asr_dev/plugin_entrypoints.md
docs/spec/cli.md
src/standard_asr/compliance.py
src/standard_asr/contract/language.py
src/standard_asr/contract/results.py
src/standard_asr/renderers.py
src/standard_asr/runtime/interface.py
src/standard_asr/runtime/streaming.py
src/standard_asr/toolchain/cli.py
src/standard_asr/toolchain/doctor.py
tests/test_asr_interface.py
tests/test_cli.py
tests/test_compliance.py
tests/test_doctor.py
tests/test_language.py
tests/test_results.py
tests/test_server.py
tests/test_streaming.py

🚧 Files skipped from review as they are similar to previous changes (7)

README.md
docs/for_app_dev/errors.md
tests/test_streaming.py
docs/for_app_dev/streaming.md
tests/test_language.py
src/standard_asr/toolchain/doctor.py
src/standard_asr/runtime/streaming.py

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/standard_asr/toolchain/cli.py (1)

815-822: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale docstring: the bridge no longer pre-gates on streaming_input.

Lines 818–822 still say the bridge "runs only for a streaming_input engine" and that an output-only engine "is reported as skipped". The code (Line 883 and the comment above it) now always calls _run_sync_bridge, letting check_sync_bridge emit a structured sync_bridge_not_applicable warning — which test_cli_compliance_run_bridge_not_applicable_for_output_only_engine asserts explicitly ("skipped sync-bridge" must not appear).

📝 Proposed docstring fix

     streaming or not). For a streaming engine
     it additionally runs ``check_streaming_param_gating`` and, when
     ``include_bridge`` is set, ``check_sync_bridge`` with ``bridge_timeout`` as
-    its total whole-session timeout (the CLI's ``--bridge-timeout``). The
-    bridge itself runs only for a ``streaming_input`` engine: it feeds bare
-    PCM frames, which an output-only engine's session cannot accept
-    (``start_transcription(audio_format=...)`` would fail on the capability
-    gate, a spurious failure about the CHECK, not the engine) -- an
-    output-only engine is reported as skipped instead.
+    its total whole-session timeout (the CLI's ``--bridge-timeout``). The
+    bridge runs for every streaming engine without a CLI-side capability
+    pre-gate: it feeds bare PCM frames, so an output-only engine's session
+    refuses establishment and ``check_sync_bridge`` (handed ``engine=``)
+    classifies that as a structured ``sync_bridge_not_applicable`` warning
+    rather than an engine failure.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/standard_asr/toolchain/cli.py` around lines 815 - 822, Update the CLI
compliance docstring near the bridge behavior description to match the current
unconditional _run_sync_bridge flow: remove the claim that streaming_input is
pre-required and that output-only engines are reported as skipped, and describe
that check_sync_bridge emits the structured sync_bridge_not_applicable warning
when the bridge is not applicable.

🧹 Nitpick comments (1)

tests/test_discovery.py (1)
1263-1266: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider runpy.run_module to drop the path lookup.

run_module targets the module by name, removing the inspect.getsourcefile call and its None guard, and it exercises the same python -m path the docstring describes.
♻️ Proposed simplification
-    module_file = inspect.getsourcefile(discovery_module)
-    assert module_file is not None
     with pytest.raises(SystemExit) as excinfo:
-        runpy.run_path(module_file, run_name="__main__")
+        runpy.run_module("standard_asr.plugins.discovery", run_name="__main__")
Note this also makes the inspect and discovery_module imports unnecessary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_discovery.py` around lines 1263 - 1266, Update the test around
runpy.run_path to use runpy.run_module with the discovery module’s importable
name and run_name="__main__", matching the documented python -m execution path.
Remove the now-unused inspect and discovery_module imports, along with the
module_file lookup and None assertion; preserve the existing SystemExit
assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_package_exports.py`:
- Around line 64-84: Add Google-style English docstrings to the public test
functions test_capability_vocabulary_is_on_the_engine_facade and
test_mode_is_one_literal_with_one_home, briefly documenting what each test
verifies. Leave their existing assertions and behavior unchanged.

---

Outside diff comments:
In `@src/standard_asr/toolchain/cli.py`:
- Around line 815-822: Update the CLI compliance docstring near the bridge
behavior description to match the current unconditional _run_sync_bridge flow:
remove the claim that streaming_input is pre-required and that output-only
engines are reported as skipped, and describe that check_sync_bridge emits the
structured sync_bridge_not_applicable warning when the bridge is not applicable.

---

Nitpick comments:
In `@tests/test_discovery.py`:
- Around line 1263-1266: Update the test around runpy.run_path to use
runpy.run_module with the discovery module’s importable name and
run_name="__main__", matching the documented python -m execution path. Remove
the now-unused inspect and discovery_module imports, along with the module_file
lookup and None assertion; preserve the existing SystemExit assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 70780f49-f3e9-420c-8b41-b8369019bf9b

📥 Commits

Reviewing files that changed from the base of the PR and between 5732f0c and 92741aa.

📒 Files selected for processing (19)

docs/for_app_dev/streaming.md
docs/spec/specification.md
src/standard_asr/__init__.py
src/standard_asr/compliance.py
src/standard_asr/contract/capabilities.py
src/standard_asr/contract/language.py
src/standard_asr/contract/results.py
src/standard_asr/plugins/discovery.py
src/standard_asr/renderers.py
src/standard_asr/runtime/gating.py
src/standard_asr/runtime/streaming.py
src/standard_asr/toolchain/cli.py
src/standard_asr/toolchain/doctor.py
tests/test_cli.py
tests/test_compliance.py
tests/test_discovery.py
tests/test_doctor.py
tests/test_package_exports.py
tests/test_results.py

🚧 Files skipped from review as they are similar to previous changes (10)

src/standard_asr/plugins/discovery.py
src/standard_asr/runtime/gating.py
docs/spec/specification.md
src/standard_asr/contract/results.py
src/standard_asr/contract/language.py
docs/for_app_dev/streaming.md
src/standard_asr/toolchain/doctor.py
src/standard_asr/runtime/streaming.py
tests/test_compliance.py
tests/test_doctor.py

+        Returns:
+            ``True`` if supported.
+        """
+        ...


coderabbitai

🧹 Nitpick comments (1)

docs/spec/cli.md (1)
72-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the duplicated timeout sentence.

Lines 84-85 repeat the per-phase timeout behavior already covered earlier in the --bridge-timeout entry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/spec/cli.md` around lines 72 - 85, Remove the repeated sentence stating
that the bridge timeout value is granted independently to each check phase,
while preserving the earlier timeout description and all other --bridge-timeout
behavior in the CLI documentation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/spec/cli.md`:
- Around line 72-85: Remove the repeated sentence stating that the bridge
timeout value is granted independently to each check phase, while preserving the
earlier timeout description and all other --bridge-timeout behavior in the CLI
documentation.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f660600b-1fe3-449b-94b0-a779b3a6ac30

📥 Commits

Reviewing files that changed from the base of the PR and between 92741aa and 65dc84e.

📒 Files selected for processing (16)

CHANGELOG.md
docs/for_app_dev/streaming.md
docs/spec/cli.md
docs/spec/specification.md
src/standard_asr/compliance.py
src/standard_asr/contract/results.py
src/standard_asr/renderers.py
src/standard_asr/runtime/interface.py
src/standard_asr/runtime/streaming.py
src/standard_asr/toolchain/cli.py
src/standard_asr/toolchain/doctor.py
tests/test_asr_interface.py
tests/test_cli.py
tests/test_compliance.py
tests/test_doctor.py
tests/test_results.py

🚧 Files skipped from review as they are similar to previous changes (8)

src/standard_asr/renderers.py
docs/for_app_dev/streaming.md
src/standard_asr/contract/results.py
docs/spec/specification.md
src/standard_asr/toolchain/doctor.py
src/standard_asr/toolchain/cli.py
tests/test_compliance.py
src/standard_asr/runtime/streaming.py

Copilot

Pull request overview

This PR is a broad “design-review sweep” to eliminate doc/code drift, tighten contract/coherence around strict gating and streaming, and make tooling/CI enforce the documented behavior across the Standard ASR protocol surface (apps, plugins, CLI, server, compliance, docs).

Changes:

Align strict-mode failures (notably candidate-language strict gating) to the structured contract (UnsupportedFeatureError) so REST/WS/CLI map them consistently as client errors.
Make timestamp-less streaming reductions explicit and safe: mark placeholder spans and ensure renderers behave deterministically (no silent fabricated timing).
Harden ecosystem tooling and gates: doctor includes the core numpy floor in conflict analysis; CLI/compliance bridge behavior becomes more robust and configurable; CI adds strict docs build and reproducible tool pinning.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_streaming.py	Adds regression tests for `StreamDeadlines` extra-forbid and streaming diagnostic code constants.
tests/test_server.py	Adds server-level regression tests for strict gating 422s and engine-fault vs request-fault error mapping.
tests/test_results.py	Adds extensive tests for placeholder timestamp signaling and SRT/VTT rendering behavior; pins exported constants.
tests/test_param_gating.py	Pins gating diagnostic code constants to their wire literals.
tests/test_package_exports.py	Verifies facade exports/aliases (e.g., `Mode` aliasing `ModeName`) and prevents drift.
tests/test_language.py	Pins language diagnostic code literals; updates strict policy failures to `UnsupportedFeatureError`.
tests/test_discovery.py	Verifies `python -m standard_asr.plugins.discovery` exits with a CLI signpost message.
tests/test_cli.py	Tests `--strict-discovery`, disables abbrev parsing, adds `--bridge-timeout` behavior and bridge classification coverage.
tests/test_audio_conversion.py	Pins conversion diagnostic code constants to wire literals.
tests/test_asr_interface.py	Ensures engine-produced `ValidationError` is wrapped as `TranscriptionError` (batch + streaming seams).
src/standard_asr/toolchain/doctor.py	Includes core numpy requirement in conflict analysis; improves classification/remediation messaging.
src/standard_asr/toolchain/cli.py	Renames discovery strictness to `--strict-discovery`, disables abbrev parsing, threads bridge timeout, improves reporting.
src/standard_asr/runtime/streaming.py	Adds streaming DIAG constants, forbids unknown `StreamDeadlines` fields, marks placeholder timestamps + emits disclosure diagnostic.
src/standard_asr/runtime/interface.py	Adds `recommended_wire_format()` to the protocol surface; wraps engine `ValidationError` as `TranscriptionError`; threads mode into language resolution.
src/standard_asr/runtime/gating.py	Aliases `Mode` to contract `ModeName`; factors gating diagnostic codes into constants/exports.
src/standard_asr/renderers.py	Updates cue selection to respect placeholder timestamp signals and avoid silent timing fabrication or cue dropping.
src/standard_asr/plugins/discovery.py	Replaces `__main__` demo prints with a loud `SystemExit` signpost to the real CLI.
src/standard_asr/contract/results.py	Removes blanket `TranscriptionResult.metadata`; introduces reserved placeholder marker + diagnostic constants.
src/standard_asr/contract/properties.py	Clarifies `engine_id` validation vs PEP 503 canonicalization in doc/field description.
src/standard_asr/contract/language.py	Adds `mode` to strict rejection typing; switches strict unreachable candidates to `UnsupportedFeatureError`; exports additional DIAG codes.
src/standard_asr/contract/identifiers.py	Collapses validation wrappers into single exported functions.
src/standard_asr/contract/exceptions.py	Corrects `InvalidSessionUseError` docstring to remove incorrect HTTP mapping claim.
src/standard_asr/contract/capabilities.py	Introduces `ModeName` as the canonical mode-domain type in the contract layer.
src/standard_asr/compliance.py	Adds protocol-required `recommended_wire_format`; improves bridge timeout validation/classification; adds minimal `SupportsCapabilities` protocol.
src/standard_asr/audio/loader.py	Removes duplicate license header block.
src/standard_asr/audio/conversion.py	Factors conversion diagnostics into constants and exports them.
src/standard_asr/init.py	Re-exports placeholder marker + diagnostic constants at package top level.
README.md	Updates messaging about compliance scope and dependency-conflict reality; references `doctor` semantics.
docs/spec/specification.md	Updates spec to match strict gating and streaming behavior; documents `recommended_wire_format()` and placeholder timestamp signaling.
docs/spec/cli.md	Documents `--strict-discovery` and `--bridge-timeout` and clarifies command behavior.
docs/mission.md	Clarifies SSRF wording and v1 limitation (DNS rebinding advisory).
docs/for_asr_dev/plugin_entrypoints.md	Updates strict discovery flag naming in docs.
docs/for_app_dev/streaming.md	Rewrites to match real streaming API behavior (managed vs manual input, error semantics, placeholder timestamps).
docs/for_app_dev/errors.md	Clarifies streaming error types and correct call-site semantics for `InvalidSessionUseError` vs `StreamClosedError`.
CHANGELOG.md	Records breaking/coherence changes (metadata removal, strict gating type, flag rename, placeholder timing behavior).
.github/workflows/mkdocs.yml	Deploy builds from locked env via uv; mirrors strict docs build; adds relevant path triggers and UV pin.
.github/workflows/ci.yml	Pins zizmor and adds PR-blocking “Docs build (strict)” job to prevent doc/code drift merges.

t41372 · 2026-08-03T10:46:19Z

    Args:
        engine: The engine under test (declares ``streaming_input``).
+        model: The model key (``engine/model``) to attribute issues to, or


Fixed in eee859e: the Args description no longer claims the engine declares streaming_input; it now states the check is deliberately capability-blind (Properties-pure self-consistency, run for any streaming-axis engine, output-only passes trivially).

Copilot