feat: P-022 first deliverable — CFG JSON seam, Rust workspace + own-ir, exact oracle#170
Conversation
…r, exact oracle Migration steps 0-1 of docs/proposals/P-022-rust-core-migration.md, gated by the differential ratchet from commit one: - ownlang/cfg_json.py + `cfg --format json`: the canonical, frozen CFG-layer oracle seam (versioned envelope, per-function symbol table in first-appearance order preserving identity structure, fixed instruction vocabulary, deterministic sorted-key dump). Human dump stays the debug default; --format gate extended (cfg: human|json; json rejected on ownir). Pinned by tests/test_cfg_json.py (19 checks) in run_tests.py. - rust/ workspace: [workspace.lints] + [profile.release] exactly as specced (unsafe_code=forbid, pedantic/nursery warn, restriction denies; panic per-binary, so NOT in the profile). crates/own-ir re-types the OwnIR fact contract with serde under the acceptance rule 'Rust accepts exactly what Python load() accepts': only load()-validated fields are typed, everything else rides a flattened extra map (additive tolerance + exact round-trip); version gate first; span/location leaf module. tests/roundtrip.rs: all 19 Python OwnIR fixtures round-trip value-for-value, version/vocabulary/bool-int/empty-identity gates. cargo test 8/8, clippy 0 issues under the full strict lints, fmt clean. - scripts/oracle_exact.py: the exact parity harness (NOT the cross-tool fuzzy oracle_compare.py — see P-022). Exit/crash gate before any output diff; canonicalized JSON (sorted keys) + path-normalized text; stderr compared; compare / snapshot / verify modes with the manifest keyed by (corpus hash, reference commit) and a staleness gate that fails verify when EITHER key changed. --selftest covers self-parity, divergence detection, the crash gate, canonicalization, and the snapshot round-trip; e2e: 20 examples x 2 surfaces = 40 records parity. Gate green: run_tests, ruff, mypy ownlang, cargo test/clippy/fmt.
|
Warning Review limit reached
Next review available in: 39 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 ignored due to path filters (1)
📒 Files selected for processing (11)
✨ 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 |
|
@coderabbitai review First code deliverable of the P-022 migration (steps 0–1). Focus areas:
Generated by Claude Code |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 170 --repo PhysShell/Own.NET --json title,body,files,additions,deletions | head -c 3000
echo "---FILES---"
gh pr diff 170 --repo PhysShell/Own.NET --name-onlyLength of output: 2855 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== cfg_json.py ==="
cat ownlang/cfg_json.py
echo "=== ownir.py (load) ==="
fd -t f "ownir.py" ownlang | xargs catLength of output: 139327 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== rust/Cargo.toml ==="
cat rust/Cargo.toml
echo "=== own-ir Cargo.toml ==="
cat rust/crates/own-ir/Cargo.toml
echo "=== lib.rs ==="
cat rust/crates/own-ir/src/lib.rs
echo "=== span.rs ==="
cat rust/crates/own-ir/src/span.rsLength of output: 14100 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== __main__.py diff-relevant (cfg command) ==="
rg -n "cfg" ownlang/__main__.py | head -50
echo "---"
rg -n -B3 -A40 "def cmd_cfg" ownlang/__main__.py
echo "=== oracle_exact.py ==="
cat scripts/oracle_exact.pyLength of output: 16587 🏁 Script executed: #!/bin/bash
set -euo pipefail
wc -l rust/crates/own-ir/tests/roundtrip.rs
sed -n '1,40p' rust/crates/own-ir/tests/roundtrip.rs
echo "..."
rg -n "allow\(clippy" rust/crates/own-ir/tests/roundtrip.rs
echo "=== fixture files ==="
fd -e json . tests -x echo {} 2>/dev/null | rg -i "ownir|fixture" | head -30Length of output: 3085 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '40,104p' rust/crates/own-ir/tests/roundtrip.rsLength of output: 2677 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "succ" ownlang/cfg.py | head -30Length of output: 800 Good — I now have enough evidence to answer all four points concretely.
1.
2.
I did not find an asymmetry in either direction. One thing to watch as the crate grows: 3.
One minor edge case worth being aware of (not necessarily a blocker): a process killed by a signal (segfault, OOM-kill) produces no matching crash-mark text and a negative/non-standard 4. Workspace lints + test-file allow Overall this is a clean, well-scoped first deliverable — the acceptance-parity claim for ✏️ Learnings added
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://cdn.github.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: a73a1f75a9
ℹ️ 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".
| pols = collect_policies(mod) | ||
| for fn in mod.functions: | ||
| cfg, _ = build_cfg(fn, rnames, sigs, pols) | ||
| cfgs = [build_cfg(fn, rnames, sigs, pols)[0] for fn in mod.functions] |
There was a problem hiding this comment.
When the new cfg --format json path builds CFGs, it omits collect_kinds(mod), so every exported symbol has resource_kind: null even when the resource declaration includes kind "...". The checker path does pass resource kinds, and cfg_json makes resource_kind part of the canonical symbol contract, so this drops semantic metadata from the frozen seam for kind-tagged resources.
Useful? React with 👍 / 👎.
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub ownir_version: Option<i64>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub components: Option<Vec<Component>>, |
There was a problem hiding this comment.
Option<Vec<_>> collapses a present JSON null into the same state as an omitted field, so OwnIr::from_json(r#"{\"components\":null}"#) succeeds and to_value() then drops the field. Python load() treats only absence as the default and rejects a present non-list via the components shape check, so this violates the “Rust accepts exactly what Python load() accepts” rule; the same absent-vs-null distinction needs to be preserved for the other load-validated optional fields too.
Useful? React with 👍 / 👎.
| now_hash = _corpus_hash(files, root) | ||
| if now_hash != manifest["corpus_hash"]: | ||
| print("STALE SNAPSHOTS: the corpus changed since `snapshot` ran " | ||
| "(corpus_hash mismatch). Regenerate before verifying.", | ||
| file=sys.stderr) | ||
| return 2 |
There was a problem hiding this comment.
verify only recomputes the corpus hash, but it never compares manifest["reference_commit"] with the current reference commit even though the manifest is documented as keyed by both. If the Python reference changes while the corpus is unchanged, this will compare candidates against stale goldens instead of returning the intended staleness error and forcing regeneration.
Useful? React with 👍 / 👎.
…ys, null parity CodeRabbit + Codex round on the first deliverable; all four confirmed: - cmd_cfg now passes collect_kinds(mod) into build_cfg like the checker path does (Codex P2): without it every kind-tagged resource exported resource_kind=null through the frozen seam. test_cfg_json pins a kind-tagged resource riding the projection (20/20). - cfg_json owns its canonicalization (CodeRabbit): new canonical_json() is the contract's textual form; __main__ prints it instead of an ad-hoc json.dumps at the call site. - oracle_exact verify now gates on BOTH manifest keys (Codex P2 + CodeRabbit): the reference key is a content hash of the reference implementation's source tree (--ref-src, default ownlang/) — not the git commit, which changes on every commit including ones that cannot affect reference behaviour. A signal death (negative returncode) counts as a crash even with no traceback text. Selftest adds the reference-edit staleness case (edit -> rc 2 -> restore -> parity). - own-ir absent-vs-null parity (Codex P2): a present JSON null is now REJECTED exactly where Python load() rejects it (reject_null deserializer on every load()-validated optional), and ACCEPTED AND PRESERVED on round-trip for the three fields Python checks with 'if x is not None' (subscription type/source_type, param effect) via a double-Option with a justified option_option allow. Two new tests pin both directions; 10/10. Gate green: run_tests, ruff, mypy ownlang, cargo test/clippy/fmt, oracle selftest.
|
All four findings folded in
Gate green across both stacks: run_tests / ruff / mypy, cargo test / clippy(0) / fmt, oracle selftest. Generated by Claude Code |
… rationale Track A (N4, no-dependency slice). - test_ownir now asserts each frontend producer (Program.cs, ownts.py) stamps the same ownir_version as the core's OWNIR_VERSION — so 'bumped the core, forgot a frontend' (or the reverse) fails loudly instead of silently mis-reading facts. This is the no-dep half of version single-sourcing; the P-022 own-ir crate will be a fourth producer to fold in (or generate from the schema). (211/211 bridge checks.) - Register: recover and record the strong pro-schema argument now that its trigger has fired. OwnIR is THE language every frontend + the core speak; a language without a formal grammar lies about itself. The schema looked redundant only while load() was the sole validator (zero-dep, no second type set). #170's crates/own-ir hand-writes Rust serde structs that must match load() — two hand-maintained validators in two languages, the exact drift formalization exists to kill. So the schema flips from a shadow of load() to the source both are generated from (serde via typify; C#/TS source-gen), collapsing the hand-synced shapes AND the four ownir_version literals into one authority. Recommendation: co-design spec/ownir.schema.json WITH the own-ir crate, not a jsonschema validator bolted onto the zero-dep core. Refs docs/notes/tech-debt-register.md (N4), P-022 #170. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
|
@coderabbitai review Final pass over the fold commit Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 170 --repo PhysShell/Own.NET --json commits --jq '.commits[-5:]'
echo "---"
git -C . log --oneline -5 2>/dev/null || true
echo "---diff---"
gh pr diff 170 --repo PhysShell/Own.NET | head -c 500Length of output: 5696 🏁 Script executed: #!/bin/bash
set -euo pipefail
git show a73a1f7..43a45a6 --stat 2>/dev/null || true
echo "---"
git diff a73a1f7 43a45a6 -- ownlang/cfg_json.py ownlang/__main__.py 2>/dev/null | head -300Length of output: 4934 🏁 Script executed: #!/bin/bash
set -euo pipefail
git diff a73a1f7 43a45a6 -- scripts/oracle_exact.py 2>/dev/nullLength of output: 7141 🏁 Script executed: #!/bin/bash
set -euo pipefail
git diff a73a1f7 43a45a6 -- rust/crates/own-ir/src/lib.rs 2>/dev/null | head -250Length of output: 8966 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "is not None" ownlang/ownir.pyLength of output: 1627 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "effect" rust/crates/own-ir/src/lib.rs | head -30Length of output: 559 🏁 Script executed: #!/bin/bash
set -euo pipefail
git diff a73a1f7 43a45a6 -- rust/crates/own-ir/tests/roundtrip.rs 2>/dev/nullLength of output: 2345
No discrepancies between the commit message and the actual diff. This closes out all four findings from the previous round. Nice, tight fold. [resolve] ᕦ(ò_óˇ)ᕤ |
… rationale Track A (N4, no-dependency slice). - test_ownir now asserts each frontend producer (Program.cs, ownts.py) stamps the same ownir_version as the core's OWNIR_VERSION — so 'bumped the core, forgot a frontend' (or the reverse) fails loudly instead of silently mis-reading facts. This is the no-dep half of version single-sourcing; the P-022 own-ir crate will be a fourth producer to fold in (or generate from the schema). (211/211 bridge checks.) - Register: recover and record the strong pro-schema argument now that its trigger has fired. OwnIR is THE language every frontend + the core speak; a language without a formal grammar lies about itself. The schema looked redundant only while load() was the sole validator (zero-dep, no second type set). #170's crates/own-ir hand-writes Rust serde structs that must match load() — two hand-maintained validators in two languages, the exact drift formalization exists to kill. So the schema flips from a shadow of load() to the source both are generated from (serde via typify; C#/TS source-gen), collapsing the hand-synced shapes AND the four ownir_version literals into one authority. Recommendation: co-design spec/ownir.schema.json WITH the own-ir crate, not a jsonschema validator bolted onto the zero-dep core. Refs docs/notes/tech-debt-register.md (N4), P-022 #170. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
… new dep (#175) * spec(ownir): add machine-readable JSON Schema, pinned to code with no new dep OwnIR now has a machine grammar to sit alongside the prose spec (spec/OwnIR.md): spec/ownir.schema.json (JSON Schema draft 2020-12, ownir_version as const:0). It mirrors the contract the Python validator (ownlang/ownir.py::load) enforces — envelope, owned-resource records, flow bodies, DI graph, reactive effects — and is shaped for the P-022 `own-ir` Rust crate to generate serde types from: the flow-op vocabulary is a `oneOf` discriminated on `op`, and the enums (resource kinds, DI lifetimes, param effects) are closed. The schema's trigger has fired: a second language-native type set (the Rust crate in #170) is arriving, so a single source both are generated from stops the two hand-maintained validators from drifting. The core cannot import a jsonschema validator (zero-dependency constraint), so instead of validating documents this binds the schema's *vocabulary* to the code's authoritative sets in tests/test_ownir.py: - resourceKind enum == _KNOWN_RESOURCE_KINDS (the load() routing set) - diLifetime enum == di.LIFETIMES - ownir_version const == OWNIR_VERSION - every flowOp const lowers through _lower_flow without the fail-loud "unknown op" raise; a bogus op the schema omits still raises Schema and validator therefore cannot drift without a red build, no dependency added. Registered in spec/README.md; tech-debt register item 3 updated to shipped. 228/228 bridge checks pass; full suite green. * style(test): sort the new ownir imports (ruff I001) The schema-consistency imports (_KNOWN_RESOURCE_KINDS, DI_LIFETIMES) tripped ruff's import-order check on the PR; merge them into the sorted block. * fix(ownir): address Codex review — null parity + bidirectional enum binds Three P2 findings from the Codex review on the schema PR: 1. Null resource types. The reference loaders (Python load(), the Rust own-ir parity tests) accept and preserve `"type": null` — load() only rejects a non-null non-string. The schema made `type`/`source_type` string-only, so it (and generated serde types) would reject documents the pipeline accepts. Widen them to ["string","null"] — maps to Option<String>. `source` widened too (load() never constrained it). 2. paramEffect never pinned. The schema advertised a closed paramEffect enum but the drift test jumped straight from diLifetime to flow ops, so it never bound. Extract the effect tuple in load() to a named authority `_PARAM_EFFECTS` and assert schema.paramEffect == it. 3. Flow-op bind was one-directional. The test only proved schema-ops lower; a handled op the schema forgot stayed green. Extract the lowerer's vocabulary to `_FLOW_OPS` (the _lower_flow `else` now distinguishes "declared-but-unhandled" from "unknown vocabulary") and assert schema.flowOp consts == _FLOW_OPS — both directions. The drive-test still lowers every declared op so a phantom set entry fails. Both _FLOW_OPS and _PARAM_EFFECTS now mirror _KNOWN_RESOURCE_KINDS: a named frozenset used by load()/_lower_flow AND bound to the schema by equality, so schema and validator cannot drift either way. 230/230 bridge checks; ruff + mypy --strict + full suite green. * fix(ownir): add event/handler/lambda to resourceRecord schema CodeRabbit (Major): the resourceRecord schema omitted the event, handler, and lambda fields that every subscription/timer/capture fact carries — event/handler are core identifying fields (finding messages, rendered output) read as strings via sub.get(), and lambda is the bool flag (ownir.py:2171) marking an un-detachable inline-lambda handler. Without them the schema-generated Rust own-ir serde types would drop the fields and lose round-trip fidelity for real facts, undermining the schema's single-source purpose. Added additive/optional, matching code semantics (event/handler string, lambda bool). 230/230 bridge checks; full suite green. --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Что и зачем
Первый код-деливерабл Rust-миграции — шаги 0–1 плана P-022, ровно как заморожено в доке («workspace skeleton +
own-irround-trip + oracle harness, gated by the differential ratchet from commit one»):cfg --format json(ownlang/cfg_json.py) — канонический CFG-seam для оракула: versioned envelope, per-function таблица символов в порядке первого появления (сохраняет identity-структуру — то, что порт ломает первым), фиксированный op-словарь, детерминированный sorted-keys дамп. Human-дамп остаётся debug-видом по умолчанию. Пин:tests/test_cfg_json.py(19 проверок) в аггрегаторе.rust/workspace +crates/own-ir—[workspace.lints]и[profile.release]в точности по P-022 (unsafe_code=forbid, restriction-denies;panicper-binary → не в профиле).own-irре-типизирует OwnIR-контракт под правилом «Rust принимает ровно то, что принимает Pythonload()»: типизированы только валидируемые поля, остальное — flattenedextra(additive-tolerance + точный round-trip); version-gate первым; span/location leaf.tests/roundtrip.rs: все 19 Python-фикстур round-trip value-for-value + гейты (версия, словари lifetime/effect, bool≠int, пустые identity).cargo test8/8, clippy 0 замечаний под полными строгими линтами, fmt clean.scripts/oracle_exact.py— exact-parity harness (НЕ fuzzyoracle_compare.py, см. P-022): exit/crash-gate до любого диффа, канонизация JSON (sorted keys) + path-normalization, stderr сравнивается; режимыcompare/snapshot/verifyс манифестом по ключу (corpus hash, reference commit) и staleness-gate на смену любого ключа.--selftestпокрывает self-parity, divergence-детект, crash-gate, канонизацию, snapshot round-trip; e2e: 20 examples × 2 поверхности = 40 записей parity.Семантика Python-ядра не изменена (только новая поверхность
cfg --format json); Python остаётся авторитетным.Тип изменения
Как проверено
python tests/run_tests.py— зелёный (добавленtest_cfg_json.py, 19/19)ruff check .иmypy— чисто (19 файлов)python scripts/oracle_exact.py --selftestPASScargo test8/8 (round-trip всех OwnIR-фикстур),cargo clippy --all-targets0 issues под строгими линтами,cargo fmt --checkcleanexamples/— 40/40 parityСвязанные issue
Нет issue. Реализует migration steps 0–1 из P-022 (#166, #168).
Чеклист
test_cfg_json.py,roundtrip.rs,oracle_exact.py --selftest__main__docstring; архитектура уже описана в P-022feat:)🤖 Generated with Claude Code
Generated by Claude Code