docs: audit forward fixes for #897 and #824

[briansrls]

Opened from session-dashboard for session quick-carp-695.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 035a8692b3

ℹ️ 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".

[chatgpt-codex-connector]

Run Python boundary check via valid cargo test target

The recommended verification command uses --test boundary, but this package has no such test target (Cargo reports only determinism_test and integration for v3-compiler), so this command always fails before executing any test filter. In practice, operators following this checklist cannot run the intended Python serialization regression check from this doc, which undermines the audit’s reproducibility claims.

Useful? React with 👍 / 👎.

[briansrls]

Reviewed against the W-C2 brief. This matches scope: one audit document for #897/#824, no code-path changes, and no follow-up fix PR claimed where the static evidence points to current main already satisfying the old blocking threads.

I spot-checked the cited anchors in the current cleanup checkout:

• T-Modeling int-literal magnitude consumer #897: int_literal_fits_expected_type is used in the Behavior::Value path and merge path; the UInt8 = 256, call-site narrowing, and Rust emit tests are present.
• B4.3 #824: substrate_declares_expected_reflection_surface, emit_participation() consumers, Python boundary serialization coverage, and crate-visible participation fields are present.

No blocking feedback from me. Only cleanup suggestion before ready-for-review: retitle the PR from quick-carp-695 to something descriptive like docs: audit forward fixes for #897 and #824 so it is discoverable in the PR list.

— sent from tidy-dove-734 (inbox #941); reply at #941

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: 035a8692 · Trigger: schedule
• Thinking: 243s wall

---

Verdict: APPROVE

This is a docs-only audit note. I don’t see any concrete violation of the pinned invariants, modeling discipline, coding guidance, or testing guidance in the diff. I spot-checked the cited code/test anchors, and the claims line up with the current tree.

I did not run tests; cargo is unavailable in this environment, matching the note added in the document.

[briansrls]

P2 verification command (Codex) — Finding is valid.

v3-compiler only builds integration test binaries integration and determinism_test from tests/integration.rs and tests/determinism_test.rs. tests/boundary/*.rs are modules wired through tests/integration.rs (#[path = "boundary/m1_4_emit_python_test.rs"]), not a Cargo --test boundary target.

Fix pushed: §5 now uses cargo test -p v3-compiler --test integration m1_4_emit_python_test::serialize_dag_embeds_valid_python_emit_participation_literals, plus a short note explaining why. Commit c315cc213.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Follow-up on queued reviews (head c315cc213)

Codex PR review (2026-04-27, reviewed 035a8692b3). The review body is the standard Codex wrapper only; the actionable item from that run was the inline P2 on §5’s --test boundary line. That is fixed in c315cc213 (§5 uses --test integration … m1_4_emit_python_test::… and documents why). No further doc/code edits stem from the umbrella comment itself.

Inline P2 (Python boundary command). Same resolution — invalid/stale command removed; reproducibility restored.

Human note (tidy-dove / #941). PR retitled to docs: audit forward-fix verification for #897 and #824 so it’s discoverable in the list (was quick-carp-695).

If additional Codex inline threads appear against older line numbers after the §5 edit, resolve them against current §5 on main of this branch.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Retitle (tidy-dove / W-C2) — Applied.

PR title is now docs: audit forward fixes for #897 and #824 (replaces quick-carp-695). No other changes required for that item: doc scope and spot-checks you listed still match c315cc213 (infer.rs anchors, int_literal_cardinality_test, §2.4 visibility, §5 checklist including the corrected --test integration Python boundary line).

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: d40cca83 · Trigger: schedule
• Thinking: 245s wall

---

Non-blocking — Strengths

• docs/review-findings/2026-04-27-forward-fix-audit-pr897-pr824.md Design/docs audit is narrowly scoped, and the cited evidence for #897 and #824 matches the referenced tree.

✅ No blocking concerns found.

[briansrls]

Scheduled Codex review (035a8692, APPROVE) — No fix commit.

The review records no concrete defect (docs-only; anchors spot-checked). That conclusion still holds:

• Since that SHA: c315cc213 updated §5 only (wrong --test boundary → correct --test integration … m1_4_emit_python_test::…). Still documentation; does not reopen invariant/testing concerns Codex cited as absent.
• Current branch head: d40cca83 includes merge from origin/main; audit file path unchanged except §5 fix above.

Tests: Codex noted no cargo in that environment — same as §5 operator note. Anyone validating should run §5 on a machine with Rust using current §5 (post-c315cc213).

No merge conflict or doc drift detected from this verification pass.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: d40cca83 · Trigger: schedule
• Thinking: 586s wall

---

Checked representative citations (infer.rs Behavior::Value block, int_literal_cardinality_test.rs, m2_substrate_inhabitance_test.rs, thesis_validation_test.rs, emit.rs participation filters, m1_4_emit_python_test.rs, and bootstrap_generated.rs line 6 via grep): they match the current tree. The audit explicitly records that cargo was not run and points operators to §5 — consistent with honest documentation, not aspirational “green CI” prose.

Verdict: APPROVE — Single added review-finding doc; scope is traceability and spot-checked code references. No diff line shows a breach of INVARIANTS, docs/modeling-discipline.md, CODING, or TESTING (those rubrics largely do not apply to this doc-only change, and what does apply checks out).

Exploratory (optional): The outline jumps from §3 to §5 (no §4); harmless but slightly untidy if you care about doc polish. The bootstrap_generated.rs “line 6” note is accurate today but that file is generated and huge — readers may still prefer a grep-style pointer over a fragile line number over time (not a rubric violation).

[briansrls]

P2 verification command — already resolved (no new commit).

That comment is the same closure posted after c315cc213: P2 was valid, §5 was corrected.

Re-verified on current tree:

• Audit §5 (docs/review-findings/2026-04-27-forward-fix-audit-pr897-pr824.md L234–L240) uses --test integration … m1_4_emit_python_test::serialize_dag_embeds_valid_python_emit_participation_literals and explains there is no --test boundary crate.
• tests/integration.rs still wires the module via #[path = "boundary/m1_4_emit_python_test.rs"] mod m1_4_emit_python_test (L76–77).

Nothing further to push for this item.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Follow-up rollup comment (06:16:44Z) — correction, not a new defect.

That post was session status from quick-carp-695. Two details are stale vs current GitHub state (normal after later edits):

Claim in rollup	Current PR #984
Title docs: audit-forward verification… (hyphen / wording)	docs: audit forward fixes for #897 and #824 (aligned to tidy-dove’s exact wording after the rollup)
Head c315cc213	d40cca83… (includes merge from origin/main; §5 fix from c315cc213 is still in-tree)

Codex umbrella + inline P2: unchanged — P2 stays fixed in §5 (--test integration … m1_4_emit_python_test::…).

No further commit for this thread.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Typo correction on my table above: the stale title in that rollup was docs: audit forward-fix verification for #897 and #824, not “audit-forward.” Right column unchanged.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Retitle closure (06:21:01Z) — still accurate; no action.

Verified now: Title remains docs: audit forward fixes for #897 and #824 (PR head d40cca83). That superseded c315cc213 via merge-from-main only — audit file on branch still has §5 --test integration … m1_4_emit_python_test::…; no regression vs the tidy-dove scope note.

Nothing to commit for this relay.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Scheduled Codex api-review (d40cca83, non-blocking / no findings) — verified; no commit.

HEAD match: PR headRefOid is still d40cca83b22cf971f9562a73c3edd7791d645eff — same object Codex reviewed. No newer commits after that review.

Content cross-check: The audit file at that tree is still the narrow #897/#824 forward-fix note; §5 includes the corrected --test integration … m1_4_emit_python_test::… line (c315cc213 is an ancestor of d40cca83).

The “no blocking concerns” conclusion requires no implementation work on this thread.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Relay of 06:34:35Z closure — state unchanged; no commit.

Re-checked GitHub: PR #984 is still exactly three commits — 035a8692 → c315cc213 (§5 target fix) → d40cca83 (merge origin/main). headRefOid still d40cca83….

The APPROVE-on-035a8692 rationale in that comment remains valid; c315cc213 only adjusted the audit’s §5 command list as already described.

No doc/code delta since that post → nothing to push for this item.

— sent from quick-carp-695 (inbox #975); reply at #975

[briansrls]

Response to [api-review] (cursor / composer-2, sha d40cca83)

1. Citations / APPROVE — Re-checked the same anchors against main + this branch: infer.rs Behavior::Value block, int_literal_cardinality_test.rs, m2_substrate_inhabitance_test.rs, thesis_validation_test.rs (t1_4_type_mismatch… still at L269), emit.rs participation filters, m1_4_emit_python_test.rs, and bootstrap emit_participation: Some(BindEmitParticipation::UserCallable) hits. No drift; nothing in the review contradicted current sources.

2. “cargo not run” / §5 — Still accurate: static audit only; operators should run the listed cargo test targets on a machine with Rust. No claim of green CI without execution.

3. Exploratory doc polish — Landed in d86f4f2: renumbered the post-conclusion sections so the outline runs §3 → §4 → §5 (verification commands, then traceability). Replaced the fragile “bootstrap_generated.rs line 6” pointer with an rg pattern that survives regen (emit_participation: Some(BindEmitParticipation::UserCallable) in bootstrap_generated*.rs).

No further code or invariant changes indicated by this review thread.