feat(parity): MOS summaries dump harness — Python↔Rust byte-exact (roadmap stage 1) - #308
Conversation
…admap stage 1) The summaries dump becomes a two-sided parity artifact, the summary-level diff the interprocedural roadmap names as stage 1's second role (and a P-022 cutover prerequisite for the inference layer): Python (authoritative): - tests/test_summaries_fixtures.py — new fixture family in the lowered-family style. Goldens pin the EXACT `python -m ownlang summaries` stdout bytes (json.dumps indent=2 sort_keys, default ensure_ascii, trailing newline). Two case sources, one golden tree: the frozen Layer 2 facts corpus swept automatically (minus the audited `excluded_lowered` ledger — an exclusion must still FAIL load() or the build goes red demanding promotion), plus 9 synthetic MOS cases (SCC cycles, return-forward chains/cycles, the extern-boundary log, overload-merge tie-breaks, sig-key vocabulary, the degraded branch, sink channels, explicit effects, non-ASCII escaping). INF-R1 is enforced per case: every dump re-runs with functions[] reversed. Rust: - own-bridge/src/mos.rs — the dump surface is now carried in full: param name/disposable, method file/line/source, and solve_with_log with the sorted unresolved-boundary log (extern param + return edges); solve() stays as the lowering-path wrapper. lookup gains the reference's non-disposable short-circuit; SCC members filter on disposable like the reference. - own-bridge/src/lower.rs — build_skeletons/merge_skeletons carry the INF-R1 tie-breaks: lexicographic-min merged param name, min (file, line) merged location. - own-bridge/src/dump.rs (new) — dump_summaries renders the document byte-identically to the CLI, with a Python-json.dumps-compatible emitter (sorted keys at every level, 2-space indent, \uXXXX ensure_ascii escaping with surrogate pairs — serde_json cannot produce that shape); a solver failure degrades to the reference's "ValueError: …" text (INF-F6). - own-bridge/tests/summaries.rs (new) — the harness: ledger/tree equality in every direction, byte-exact replay of all 35 goldens, determinism re-run, ≥35 case floor, the degraded branch pinned. tolerant_unknown_kind is the only exclusion (load()-door rejection, #294 OD-2 door placement — its rejection text stays pinned by the lowered family). Docs: spec/Inference.md §8 points at the harness; the roadmap's stage-1 parity bullet is marked shipped; stale "#294 stays open" note in own-bridge lib.rs corrected. Verified: python tests/run_tests.py (all suites incl. the new family: 35 cases, 1 degraded), ruff, mypy, cargo fmt --check, clippy (no new warnings), cargo test (29 suites / 116 tests green). Red-checked: a mutated golden fails the Rust harness with the byte-diff message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds a Rust MOS summary serializer with Python-compatible formatting, carries metadata and unresolved boundaries through resolution, and introduces Python/Rust golden parity harnesses with synthetic and lowered fixtures. ChangesMOS summary parity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PythonFixtures
participant GoldenFiles
participant RustParityTest
participant OwnIr
participant SummaryDump
PythonFixtures->>GoldenFiles: write summary goldens
RustParityTest->>OwnIr: load facts.json
OwnIr->>SummaryDump: provide typed facts
SummaryDump-->>RustParityTest: emit summary bytes
RustParityTest->>GoldenFiles: compare exact bytes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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: d5f3b6933c
ℹ️ 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".
| let root = root.as_object().cloned().unwrap_or_default(); | ||
| // Python: `str(facts.get("module", "?"))` / `facts.get("functions", [])` | ||
| // (a present non-list reads as empty). | ||
| let module = root.get("module").map_or_else(|| "?".to_owned(), py_str); |
There was a problem hiding this comment.
When an accepted OwnIR document has a container-valued module (the Python loader does not validate this field, and Rust preserves it in the flattened root metadata), this call breaks the advertised byte-exact parity because py_str uses JSON formatting for containers rather than Python's str. For example, "module": ["M"] is rendered as "['M']" by Python but as "[\"M\"]" by Rust. Either reproduce Python container formatting here or reject the same values at both input doors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified — the divergence is real (load() accepts any module type; Python str(["M"]) → "['M']" vs JSON text here), but neither of the two proposed resolutions is sound to take inside this PR:
- Reproduce Python
str(): unattainable for dicts under the current representation.serde_json's map (nopreserve_order) re-sorts keys at parse time, while Pythonstr()preserves insertion order —{"b":1,"a":2}diverges no matter how faithful the repr emulator is. Enablingpreserve_orderis a workspace-wide serialization-semantics change, disproportionate for a shape with no producer. - Reject at both doors: the class is wider than
module—functions[].name,file, and callcallee/sigare allstr()-ified from unvalidated JSON, on the Layer 2 surface too (this predates the PR; it is the long-documentedpy_strcaveat inlower.rs). Tighteningload()piecemeal for one field is cosmetic; aligning the whole door is an authoritative-contract change of the Bridge contract OD-1/2/3: pin the tolerant door — direct check_facts() diverges from load() (unknown kind fallback, line coercion) #294 kind and deserves its own tracked decision, not a rider on this harness.
What this PR now does instead (2a34c7a): the scope is stated explicitly at the dump door — the parity contract covers scalar metadata, matching the crate's "restricted to the behavior the shared fixtures exercise" charter — and the property that keeps it honest is fail-loud: the Python generator would happily pin "['M']" into a golden, so any future fixture that smuggles a container in turns the Rust harness red on the byte diff rather than diverging silently.
Generated by Claude Code
The reference str()-ifies raw metadata (module, functions[].name/file) without type validation on either door; a container there renders as Python repr vs JSON text here — a shape with no producer that cannot be emulated faithfully for dicts (serde_json re-sorts map keys, Python str() keeps insertion order). Keep it out of the parity contract explicitly: the harness pins scalar-metadata corpora only and goes red on the byte diff if such a fixture ever lands, rather than diverging silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
…own) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_summaries_fixtures.py (2)
216-223: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid recomputing
_projectjust to readdegraded.
_projectalready ran once per case in the main loop (line 189), each call doing two solver+render passes (expected + permuted). This second pass reruns it again per case purely to checkdegraded, doubling the total solve/render work for every case. Since this code only executes once every case already succeeded,degradedcan be derived fromexpectedinline in the main loop instead.♻️ Proposed fix
+ n_degraded = 0 for case, facts_path in sorted(plan.items()): golden_path = os.path.join(FIXDIR, f"{case}.summaries.json") try: expected, permuted = _project(facts_path) except OwnIRError as e: fails.append(f"{case}: load() rejected its facts ({e}) — a " f"rejected case must be in excluded_lowered, not " f"silently skipped") continue if permuted != expected: fails.append(f"{case}: dump is not byte-identical under " f"functions[] permutation (INF-R1)") continue if not os.path.exists(golden_path): fails.append(f"{case}: golden missing; regenerate with " f"'python tests/test_summaries_fixtures.py --write'") continue with open(golden_path, encoding="utf-8") as f: actual = f.read() if actual != expected: fails.append(f"{case}: golden is stale (the solver or the dump " f"changed); regenerate with " f"'python tests/test_summaries_fixtures.py --write'") + continue + if json.loads(expected).get("degraded") is not None: + n_degraded += 1 for orphan in sorted(_goldens() - set(plan)): fails.append(f"{orphan}: orphaned golden (not a planned case); remove " f"it or restore the case (manifest/facts)") if fails: for f_ in fails: print(f"FAIL: summaries fixture {f_}") return 1 - n_degraded = 0 - for facts_path in plan.values(): - if json.loads(_project(facts_path)[0]).get("degraded") is not None: - n_degraded += 1 print(f"summaries (MOS parity) fixtures OK: {len(plan)} cases " f"({n_degraded} degraded, {len(plan) - n_degraded} solved) " f"verified in sync") return 0🤖 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_summaries_fixtures.py` around lines 216 - 223, Remove the second per-case _project call used to count degraded fixtures. In the main loop around the existing _project invocation, derive and accumulate the degraded count directly from that case’s expected result, then use the accumulated value in the final summary while preserving the existing verification behavior.
149-158: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the already-computed
loweredset instead of re-listing the directory.
loweredwas computed once at line 122 via_disk_cases(LOWDIR, ".facts.json"); recomputing it here per synthetic case re-hits the filesystem for no benefit.♻️ Proposed fix
- elif name in _disk_cases(LOWDIR, ".facts.json"): + elif name in lowered:🤖 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_summaries_fixtures.py` around lines 149 - 158, In the synthetic-case validation loop, replace the per-case _disk_cases(LOWDIR, ".facts.json") call with the already-computed lowered set from the surrounding test setup. Keep the existing shadow-detection condition and error message unchanged while avoiding repeated filesystem scans.
🤖 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 `@tests/test_summaries_fixtures.py`:
- Around line 216-223: Remove the second per-case _project call used to count
degraded fixtures. In the main loop around the existing _project invocation,
derive and accumulate the degraded count directly from that case’s expected
result, then use the accumulated value in the final summary while preserving the
existing verification behavior.
- Around line 149-158: In the synthetic-case validation loop, replace the
per-case _disk_cases(LOWDIR, ".facts.json") call with the already-computed
lowered set from the surrounding test setup. Keep the existing shadow-detection
condition and error message unchanged while avoiding repeated filesystem scans.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 61a10316-e281-4cae-b89b-1fca1c5f4abb
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (54)
docs/notes/interprocedural-roadmap.mdrust/crates/own-bridge/Cargo.tomlrust/crates/own-bridge/src/dump.rsrust/crates/own-bridge/src/lib.rsrust/crates/own-bridge/src/lower.rsrust/crates/own-bridge/src/mos.rsrust/crates/own-bridge/tests/summaries.rsspec/Inference.mdtests/fixtures/summaries/alias_join_cases.summaries.jsontests/fixtures/summaries/flow_kill_on_rebind.summaries.jsontests/fixtures/summaries/flow_unmapped_refs.summaries.jsontests/fixtures/summaries/fn_params_ordering.summaries.jsontests/fixtures/summaries/handles_global_counters.summaries.jsontests/fixtures/summaries/handles_null_metadata.summaries.jsontests/fixtures/summaries/hoist_neg_early_return.summaries.jsontests/fixtures/summaries/hoist_neg_nested_depth.summaries.jsontests/fixtures/summaries/hoist_neg_while_body.summaries.jsontests/fixtures/summaries/hoist_pool_kind.summaries.jsontests/fixtures/summaries/hoist_positive_release.summaries.jsontests/fixtures/summaries/hoist_positive_use_only.summaries.jsontests/fixtures/summaries/lines_preserved.summaries.jsontests/fixtures/summaries/manifest.jsontests/fixtures/summaries/mos_call_channel_overload_sig.summaries.jsontests/fixtures/summaries/mos_call_direct_consume.summaries.jsontests/fixtures/summaries/mos_call_unknown_drop.summaries.jsontests/fixtures/summaries/mos_fresh_mint.summaries.jsontests/fixtures/summaries/mos_killsite_toplevel.summaries.jsontests/fixtures/summaries/mos_untrack_inbranch.summaries.jsontests/fixtures/summaries/mosdump_degraded_duplicate_key.facts.jsontests/fixtures/summaries/mosdump_degraded_duplicate_key.summaries.jsontests/fixtures/summaries/mosdump_explicit_effects.facts.jsontests/fixtures/summaries/mosdump_explicit_effects.summaries.jsontests/fixtures/summaries/mosdump_nonascii_escaping.facts.jsontests/fixtures/summaries/mosdump_nonascii_escaping.summaries.jsontests/fixtures/summaries/mosdump_overload_merge_tiebreaks.facts.jsontests/fixtures/summaries/mosdump_overload_merge_tiebreaks.summaries.jsontests/fixtures/summaries/mosdump_partial_and_guarded.facts.jsontests/fixtures/summaries/mosdump_partial_and_guarded.summaries.jsontests/fixtures/summaries/mosdump_return_chains.facts.jsontests/fixtures/summaries/mosdump_return_chains.summaries.jsontests/fixtures/summaries/mosdump_scc_cycles.facts.jsontests/fixtures/summaries/mosdump_scc_cycles.summaries.jsontests/fixtures/summaries/mosdump_sig_vocabulary.facts.jsontests/fixtures/summaries/mosdump_sig_vocabulary.summaries.jsontests/fixtures/summaries/mosdump_sink_channels.facts.jsontests/fixtures/summaries/mosdump_sink_channels.summaries.jsontests/fixtures/summaries/routing_r1_unresolved.summaries.jsontests/fixtures/summaries/routing_r2_subscribe_self.summaries.jsontests/fixtures/summaries/routing_r3_capture_static.summaries.jsontests/fixtures/summaries/routing_r4_returned_fresh.summaries.jsontests/fixtures/summaries/routing_r5_di_capture.summaries.jsontests/fixtures/summaries/routing_r6_token_kinds.summaries.jsontests/fixtures/summaries/vocab_unknown_op.summaries.jsontests/test_summaries_fixtures.py
…lowered set Both CodeRabbit nitpicks on the summaries fixture harness: the degraded counter re-ran _project (two solver+render passes) per case after the main loop had already produced the same bytes — derive it from the verified dump instead; and the synthetic shadow check re-listed the lowered directory per case where the already-computed set serves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
Arbiter P2 follow-up on the Codex finding: the crate-level doc and the public dump_summaries promised unqualified byte-parity 'on the same facts' while the domain boundary lived only on the private implementation — the public reader had to excavate a private module to learn what 'same' excludes. The public contract now states the shared parity domain itself (metadata consumed through Python str() must be JSON scalars; container-valued metadata diverges without a runtime error and stays a #294-class door decision), and the dump.rs header's BYTE-EXACT claim carries the same qualifier. No implementation change: rejecting containers Rust-side would open a new door asymmetry (Python's dump accepts and reprs them), the opposite of what #294 closed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
2 participants
Что и зачем
Дамп MOS-сводок (
python -m ownlang summaries) становится двусторонним паритетным артефактом — тем самым «нормализованным диффом сводок Python↔Rust» из роадмапа (этап 1, §2), который проверяет порт инференции на уровне сводок, а не только конечных диагностик. Rust-сторона (own_bridge::dump_summaries) воспроизводит байт-в-байт stdout эталонного CLI на 35 кейсах: замороженный Layer-2 корпус фактов (26, заметается автоматически) + 9 синтетических MOS-кейсов (SCC-циклы, цепочки/циклы forward-return, extern-лог, тай-брейки слияния перегрузок, sig-словарь ключей, достижимая из фактов веткаdegraded, sink-каналы, явные эффекты, non-ASCII экранирование).Ключевые части:
tests/test_summaries_fixtures.py— семейство фикстур в стиле lowered-семейства; голдены пинят точные байты stdout CLI (json.dumps(indent=2, sort_keys=True), дефолтныйensure_ascii, trailing newline); INF-R1 проверяется на каждом кейсе (передёргиваниеfunctions[]→ те же байты). Исключение одно —tolerant_unknown_kind(дверьload(), остаток Bridge contract OD-1/2/3: pin the tolerant door — direct check_facts() diverges from load() (unknown kind fallback, line coercion) #294 OD-2): аудируемая запись в манифесте, которая краснеет, если дверь начнёт его принимать.own-bridge/src/mos.rsнесёт полную дамп-поверхность (name/disposable/file/line/source +solve_with_logс отсортированным extern-логом);lower.rs— тай-брейки слияния (lexicographic-min имя параметра, min(file, line)); новыйdump.rs— эмиттер, байт-совместимый с Pythonjson.dumps(sorted keys,\uXXXX-эскейпы с суррогатными парами);degradedмапится в эталонный текст"ValueError: …".own-bridge/tests/summaries.rs— леджерная сверка во все стороны (пропавший/протухший/осиротевший голден, фантомное исключение — красный билд), детерминизм, пол ≥35 кейсов, пиновка веткиdegraded.Тип изменения
Как проверено
python tests/run_tests.py— все сьюты, включая новое семейство (35 кейсов, 1 degraded)ruff check .иmypypython scripts/<...>.py --selftest) — скрипты не затронутыcargo fmt --check,cargo clippy --all-targets(без новых предупреждений),cargo test— 29 сьютов / 116 тестовdiff)d5f3b69)Связанные issue
Refs #262 (P-022 cutover: паритет уровня сводок — предпосылка разморозки), #294 (размещение дверей load/lower — зафиксировано леджером исключений), #304 (интерпроцедуральный трекер).
Чеклист
feat:,fix:,docs:…)Generated by Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation