[MrDongsls]

Required human contributor answers

1. What outcome are you trying to achieve?

   Close the two d3 package-gate gaps the Vela: serve Vela 2.0 0.3B through a Python reference worker #151 review flagged on main, plus the bytecode-cache gap confirmed on this PR.

2. What is the highest-leverage variable for that outcome?

   The import gate must bind the code that actually executes.

3. How will this work move that variable rather than create busywork?

   Port the already-reviewed vela fixes instead of redesigning anything.

4. Who is the customer or user, and what do they actually need?

   d3 users who need the verified runtime to be the executed runtime.

After answering the four questions: is a PR needed at all to achieve this outcome?

Yes, the gaps only matter once fixed on main.

For a bug fix: link actual observed failure evidence.

At main, verify_runtime_files silently accepts a package containing an unlisted torch.pyc, a foreign directory stays ahead of the package root when the root is already on sys.path at a later position, and an existing __pycache__ under the package root can shadow the verified sources with an unchecked-hash cache. The new regression tests demonstrate all three at this head. Provenance: the two P1s the Codex review raised on #151, and the bytecode-cache P1 the #156 Codex review raised and psv666 independently reproduced.

Purpose

Port the three package-gate fixes from the vela worker in #151: the package-root refusal now covers every importable file form instead of only .py, the package root is reliably forced to the front of sys.path, and an existing __pycache__ under the package root is refused before import (the runtime import itself runs with bytecode writing disabled, so the gate cannot trip on a cache of its own making). Docs are updated to match. No behavior change for an honest single-package process. Valen does not use this import pattern, so it is out of scope.

Test Plan

PYTHONPATH=src .venv/bin/python -m pytest tests/d3 tests/valen tests/frontend -q
PYTHONPATH=src .venv/bin/python -m compileall -q src/models/d3 tests/d3
.venv/bin/mkdocs build --strict

System1-Omni Version / Commit: 2b262f74 on fix/d3-package-gate, cut from main @ e66ff349

Test Result

87 passed across tests/d3, tests/valen and tests/frontend, with compileall and mkdocs build --strict green at this head. 5 new regression items over three test functions cover the three fixes: test_sourceless_and_extension_module_files_are_refused (parametrized ×3), test_the_package_root_is_forced_to_the_front_of_sys_path, and test_existing_bytecode_caches_in_the_package_root_are_refused. Environment: x86 WSL2, Python 3.12.3. Not covered: weights verification, unchanged by this PR.

Demo / evidence

See the evidence section above. Video: N/A.

Self-review

• I have reviewed the full diff and addressed the issues I found.
• I have checked that the change follows the project's architecture and stays focused on the stated purpose.
• I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
• I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-21T15:28:39.582377Z	031a226	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 031a2267a2

ℹ️ 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]

Reject bytecode caches below the package root

When a package contains a valid unchecked-hash __pycache__/d3_runtime.<cache_tag>.pyc, this direct-file check never examines it because it only sees the containing directory; CPython then loads that cache in preference to the verified d3_runtime.py. I reproduced verify_runtime_files succeeding while the subsequent import executed code from such a cache, so the full-hash gate still does not bind the code that runs. Reject or validate runtime bytecode caches as well as root-level .pyc files.

Useful? React with 👍 / 👎.

[hsliuustc0106]

First review at head 031a2267a2 (base main; NON-draft; 4 files, +68/−15).

This PR ports the two package-gate fixes from the Vela worker (#151) to the D3 reference worker, closing the D3-side gap my #151 review noted. Verified in the diff and by execution:

• The runtime gate now refuses every importable suffix the runtime table does not name — source files, .pyc, extension modules (via importlib.machinery.SOURCE_SUFFIXES/EXTENSION_SUFFIXES), and __init__.py packages — where previously only *.py and __init__.py were caught. The refusal semantics (package root first on sys.path → shadowing risk) are unchanged and correctly described.
• Docs updated consistently across all three surfaces (recipe/d3/reference.md, src/models/d3/README.md, engine.py docstring) — the previously-enumerated forms were replaced with the suffix-set description rather than a new enumeration that could drift again.

Locally run at this head: PYTHONPATH=src pytest tests/d3 → all pass (the manifest-gate tests at tests/d3/test_engine_manifest.py:171-194 pin the widened refusals — transformers.py, .pyc-style names, and the fla package are all covered). Fork CI at this exact head: rust/build/cuda-contract/benchmarks green. Template complete per the #142 standard.

No actionable findings. APPROVED. Unverified: pinned-package fidelity needs the ~52 GB download (the gate's hash table is transcribed and enforced at startup); Rust suite covered by exact-head CI. Provenance: review skill at main@e66ff34 (SHA-256 58be3bc6…dde108); remote head re-pinned immediately before posting.

[psv666]

[P1] The verified source can still be bypassed by an existing bytecode cache.

I independently reproduced the existing finding at unchanged head 031a2267a2229a4d6eef9981942e7347937991a5. This confirms that finding rather than introducing a separate issue.

The direct-file check allows __pycache__/d3_runtime.<cache_tag>.pyc. With a valid py_compile.PycInvalidationMode.UNCHECKED_HASH cache, CPython can import that cache even though the restored d3_runtime.py matches the manifest. My CPU probe produced:

verify_runtime_files -> ["d3_runtime.py"]  # succeeds
verified source marker -> "verified_source"
subsequent import marker -> "unverified_bytecode"

The gate therefore does not establish that the verified code is the code that runs. Please reject/validate the relevant existing caches before import, or load the verified source explicitly. Disabling bytecode writes alone does not prevent reading an existing cache. A regression should populate an unchecked-hash cache with different code, restore the manifest-matching source, and assert that unverified code cannot execute.

Personally run on this head: 86 D3/Valen CPU tests passed, strict documentation build passed, plus the failing gate/import probe above. No weights, model inference or GPU validation was used; this package-loading defect is reproducible on CPU.

Review scope and provenance

• Reviewed head: 031a2267a2229a4d6eef9981942e7347937991a5; checked unchanged immediately before publication.
• PR API base (main): 32bdd2aade0e0f3f0452d04ca5022fcaaa4783c9; diff merge-base: e66ff34996760a84ad56690f0ae4cf434e669e65. Validation above used an isolated archive of the reviewed head.
• Authored code/test diff: +60/−9; total text diff: +68/−15 (generated fixtures/docs counted only in total).
• Repository InferMatrixLab/system1-omni (ID 1386454853). Applied .agents/skills/system1-omni-review/SKILL.md from trusted main@5ba6a3ba668cb33028eb0bce2c6d17cdc53aebe7, SHA-256 ae2dfac116376ae2ec41f65122cc1bf5aea10751b8b907198a56154e5d67230e. Loaded references: repository contracts, architecture, CONTRIBUTING, self-review artifact/demo rules, CI/docs workflows and the affected component documentation.

[MrDongsls]

Fixed in 2b262f7. verify_runtime_files now refuses an existing __pycache__ under the runtime root before the import, and the runtime import itself runs with bytecode writing disabled so a verified import can never leave a cache the next gate run would reject. The regression compiles different code into an unchecked-hash __pycache__/d3_runtime.<cache_tag>.pyc, restores the manifest-matching source, and asserts the gate refuses it.

[hsliuustc0106]

Review verdict: COMMENTED at 2b262f742a7edeef17d38bbd93637a238ce323d4 (origin/pr/156 = gh pr view 156 --json headRefOid = the commit 2b262f74). Non-draft, OPEN, title unchanged. Prior review: COMMENTED at 031a2267a (2026-10-21) — that review found no actionable findings (its text concluded "No actionable findings. APPROVED.", posted with a COMMENTED state).

Delta isolation

git log --first-parent 031a2267a..2b262f742 contains exactly one commit: 2b262f74 "d3: refuse existing bytecode caches at the runtime gate" (Mr.Dong, 2026-10-24; 4 files, +47/−16). Parent check: 2b262f74^ == 031a2267a byte-identical (exact SHA) — the previously reviewed port was not rebased or rewritten. All SHAs verified with git cat-file -e. Read in full (git show).

The new commit: bytecode-cache refusal at the D3 runtime gate

The third port of the shared fix (#151 origin, #152 parallel vela port). Reviewed in full:

• What is refused: any entry named __pycache__ in the package root — file or directory, checked before the importable-suffix chain (src/models/d3/engine.py:134-135), message "unexpected bytecode cache __pycache__/ in the package root", raised through the existing ValueError("runtime verification failed: …") channel (engine.py:141). Refusal set, gate position (pre-import) and error channel are identical in semantics to #151/#152; the suffix set is the same content-equivalent SOURCE | {.pyc} | EXTENSION union.
• Self-consistency guard: the import d3_runtime in D3Executor is wrapped in sys.dont_write_bytecode = True with a finally-restore (engine.py:188-196), so the gate's own import cannot write the cache the next gate run refuses. I verified the guard covers the only direct import of the runtime in src/ and recipe/ — unlike the vela recipes, recipe/d3/compare_reference.py does not import d3_runtime directly, so #151/#152's recipe-tool poisoning path does not exist here.
• Tests: new test_existing_bytecode_caches_in_the_package_root_are_refused (tests/d3/test_engine_manifest.py:199-216) — compiles an UNCHECKED_HASH cache from poisoned source (os._exit(0)), restores the manifest-matching source, expects the ValueError; the test comment states the bypass precisely ("CPython imports an unchecked-hash … .pyc in place of the verified source"). The layout test no longer seeds a __pycache__. Local run at this head: pytest tests/d3 -q → 58 passed.
• Docs: recipe, model README and docstring updated on all three surfaces, consistent with the prior port's style.

Disclosed impact: refusing legitimate precompiled deployments with root bytecode caches is disclosed in recipe/d3/reference.md and src/models/d3/README.md; for a pinned-checkpoint threat model that is the right default. One wording nit (note below): the docs say the refusal targets "stale bytecode", while the concrete bypass is unchecked-hash caches, which execute regardless of freshness.

Cross-PR status (shared fix — read together with #151 and #152): semantically identical port — same refusal set and ordering, same gate phase, same dont_write_bytecode guard pattern, same poisoned-cache test shape — with D3-appropriate message text and no verify_no_foreign_runtime (vela-only pre-import check, correctly absent). Message string and test details intentionally differ from both vela variants; no behavioral divergence.

Prior-finding status

• Prior review (2026-10-21, 031a2267a): no open findings — both ported fixes verified, docs consistent, template complete per the #142 standard, fork CI green at that exact head. Nothing to re-verify as carried; this delta is the only new content since.

Body / evidence check at the new head

The head is correctly re-pinned (2b262f74 on fix/d3-package-gate, cut from main @ e66ff349 — merge-base verified) and the Test Result's headline was re-run at this head: locally reproduced exactly (87 passed across tests/d3 + tests/valen + tests/frontend, compileall/mkdocs author-attested). Template answers, bug-fix evidence and self-review checkboxes all present. However, the narrative still describes the prior head's scope — see the P3 below.

CI at the exact head (observed)

gh pr checks 156: rust, build, cuda-contract, benchmarks pass (1m58s / 36s / 6s / 34s); deploy skips. Head SHA verified through the Actions API: both CI (run 40000674902) and Docs (run 40000674901) report head_sha = 2b262f742a7e…, conclusion success, completed (runs live on InferMatrixLab/system1-omni, which mirrors this repo; the fork MrDongsls/system1-omni has no Actions runs at this SHA).

Local checks (run at a worktree of 2b262f742; no cargo/GPU on this host)

• PYTHONPATH=. python -m pytest tests/benchmarks -q → 58 passed.
• PYTHONPATH=src python -m pytest tests/d3 -q → 58 passed.
• PYTHONPATH=src python -m pytest tests/d3 tests/valen tests/frontend -q → 87 passed (matches the body's head-pinned claim exactly).
• Worktree removed after.

Findings

• P3 — body and title no longer describe the head. The title says "port the two package-gate fixes"; the Purpose section, the bug-fix evidence paragraph and the Test Result ("4 new regression tests cover the two fixes") all describe the prior head 031a2267a. The head adds a third fix (bytecode-cache refusal) with a fifth regression item: versus base e66ff349 the diff now adds three test functions — test_sourceless_and_extension_module_files_are_refused (parametrized ×3), test_the_package_root_is_forced_to_the_front_of_sys_path, and the new test_existing_bytecode_caches_in_the_package_root_are_refused — i.e. 5 items over three fixes. The unverified line ("weights verification, unchanged by this PR") remains true. Consequence: a reader approving off the body would not know the PR now also changes what packages the D3 gate refuses (a stricter startup contract for anyone with caches in a served package). A body refresh — title optional, Purpose/Test Result/evidence required — closes this; no code change implied.

Notes (informational):

• Doc wording in recipe/d3/reference.md (~line 117), src/models/d3/README.md (~line 31) and the engine.py docstring (~line 114) attributes the refusal to "stale bytecode … in place of the verified source"; the actual bypass demonstrated by the test is an unchecked-hash cache, which need not be stale (a fresh-looking unchecked cache executes without consulting the source). The test comment gets it exactly right; tightening the doc wording would prevent a future reader from re-deriving the wrong threat model.
• Root-scoped refusal is sound for the shadowing threat model (a nested __pycache__ without importable sources beside it is not importable by name); the D3 gate does not scan nested code — same scope as #151, weaker than #152's branch-specific carries_code. Pre-existing shape, unchanged here.

Unverified

• Pinned-package fidelity (the 9-file table transcription and the real D3 checkpoint) still requires the full download — unchanged from the prior review and unchanged by this delta; the refusal logic itself is pinned by executable tests run locally at the exact head.

Verdict

COMMENTED. The delta is a faithful, self-consistent port of the shared bytecode-cache fix — semantically identical to #151/#152, correctly guarded against its own strictness, pinned by a precise regression test, and green locally (58/87) and in CI at the exact head. The prior review had no open findings; the only new item is the P3 body/title staleness, which a description refresh resolves. With that refresh this is approve-shaped.

[MrDongsls]

P3 addressed — body refreshed for the head 2b262f74 (no code change, head unchanged): Purpose, the bug-fix evidence paragraph and the Test Result now describe the three fixes and the five regression items (the three new test functions vs base e66ff349 re-checked locally: test_sourceless_and_extension_module_files_are_refused ×3, test_the_package_root_is_forced_to_the_front_of_sys_path, test_existing_bytecode_caches_in_the_package_root_are_refused). The opening answer now also names the bytecode-cache gap so the scope statement matches the head. Title left as-is per "title optional".