Repository navigation
knowledge: validate NInfer state and completion contracts - #325
Conversation
Extend the existing benchmark-design owner with fixed-source coupled-state, accepted-prefix and completion-denominator validation guidance. Retain source-only candidate boundaries and the historical report's distinct execution revision. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
Regenerate coverage and packaged skill discovery from actual source d356da0. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Skill evaluationPLANNED: not executed; there are no task outcomes yet.
Author verification steps (nonpaid; not yet run)
python tools/build_skill_index.py --source-revision d648f25732dee1c974da62a94c46a642ad52cef8 --checkThe declared source pin matches this head's canonical library bytes. The command has not run: retain its exit code and full drift/success output. It checks the generated projection, not claim truth or skill efficacy.
Evidence and logs · Retention: 30 days. Plan bindings, budget, frozen exam and advisory detailsAdvisory: obtain missing evidenceRetained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
Model/settings:
Selection/task metadata (not a relevance verdict): {"adapter":"audit","basis":"trusted_path_mapping","matched_paths":["src/inferrsi/data/skill_library/methods/inference-benchmark-design/SKILL.md","src/inferrsi/data/skill_library/methods/inference-benchmark-design/metadata.json","src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/ninfer-source.json","src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/ninfer-state-and-completion-contracts.md"],"tasks":[{"evidence_scope":"http_fixed_workload","id":"audit-threshold","request":"Audit the fixed input report using its contract. Compute the median of per-trial request means, including every request in each trial, and the percentage reduction relative to the baseline aggregate.","split":"holdout"}]}Automatic execution / cumulative PR budget: {"reason":"Trusted configuration or pricing missing, invalid or expired","remaining_usd":"unknown","reserved_or_unresolved_usd":"unknown","spent_or_legacy_usd":"unknown","state":"blocked"}Projection check selection (not an executed result): {"declared_in":"src/inferrsi/data/repo_packs/inferrsi/knowledge/README.md","source_revision":"d648f25732dee1c974da62a94c46a642ad52cef8","status":"ready"}Provider/settings: Uncovered / unsupported paths (88)
|
Retain original state and completion evidence with all current-main canonical, applicability, resource-layer and release additions before regenerating projections. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Regenerate the native coverage and catalog from actual source M 7045a9e. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Make the reference matrix source row attributable to the registered benchmark owner. Retain its hardware applicability exclusion and sources. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Regenerate coverage and packaged skill discovery from actual source bad68ff. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
|
This PR adds a NInfer source ledger and a state-restoration/completion-validation reference to the existing Reviewed commit: cbbee59 |
|
Review at head
Not verified by me: the upstream source pins in |
|
Re-review at head No new commits since the prior short-SHA note. Isolation content (NInfer ledger/reference) still looks coherent, but blockers remain:
Not verified by me: upstream source pins in Reviewed commit: |
Merge main after the Infera, Xe-Forge and DeepSeek sources landed. Xe-Forge and NInfer both extend inference-benchmark-design; keep main's additions first, then the NInfer tags, source, applicability clauses, failure modes, recommendation, limitation, readsets and matrix rows. Generated projections keep main's bytes until they are regenerated from this commit. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Run the current main writers on the committed combined source snapshot. Retain the full registered catalog and source mappings. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
There was a problem hiding this comment.
PR review verdict — DO NOT PASS
Reviewed base e16a5c2087fcd9cc4298794b4831e558eaefff46 and head 8fd68009df7057d2be063e7bc0d2c9a5dc7511c0 (full diff; pr/prior-reviews.md empty; previous head unset). This review does not approve.
Blocker
The NInfer validation contract is not reviewable from the seeded diff, so required evidence for this knowledge change is insufficient.
pr/diff.patchis 1575 lines. Readable hunks stop insrc/inferrsi/data/skill_library/methods/inference-benchmark-design/metadata.jsonat thefailure_modescontext (patch line 1573, starting"Raw throughput is optimized in place o…"). Patch line 1574 is one oversized line with no readable text; line 1575 is empty.- There is no readable
diff --gitbody for the two files the index, matrices, andSKILL.mdcite as the contract:src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/ninfer-state-and-completion-contracts.md(packagedsource_filessha2562b3de210d1790eec92319addadf0e38fcd1529fa2f23b5cddf96a789b783f4ac)src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/ninfer-source.json(sha256ccda0fc6781e3c4dceb41521ab952445840b4afcc9f5a7d968857f76e8db6272)
- Trigger: those bodies are what would substantiate the 27-file readset, rights/exclusions, per-file hashes, the upstream path
docs/performance/qwen3.8-27b.mdnext to the Qwen3.5 claim, and a historical execution revision distinct from1882c6adbc98043fcf89f3589aa5addb3742777a. I could not separate "files absent from the commit" from "diff tail collapsed into unreadable lines." - Expected correction: land those reference files (and the rest of the
metadata.jsondelta) as normal diff text at the indexed hashes and push. I will re-read that head.
Verified on the readable prefix
docs/reference-matrix.mdanddocs/searchspace-matrix.mdkeep the slice at source-derived candidate / empirical not_run / efficacy not_evaluated, with historical author results on a separate execution revision and no new implementation, backend, or measured coverage.- The
metadata.jsonsource entry lists 27 paths at NInfer1882c6adbc98043fcf89f3589aa5addb3742777a, license Apache-2.0, matching the matrix "27-file readset" wording. The same pin and path list are copied intoinference-benchmark-design.json. SKILL.md(coupled checkpoint, same-physical-block oracle, zero accepted drafts versus zero adopted inputs, logical commit versus device retirement, steady intervals versus complete-cohort denominators) matches the packaged failure modes, refusal, and added recommendation.docs/skill-coverage.mddigest issha256:e26cf7b1c819cfe6381ac0cba56b3bfe1efca71f8fa97453d110c47602fe954b; inference-benchmark-design advice 6→7 and readsets 9→11, including the NInfer link.- Sampled knowledge records other than
inference-benchmark-designonly move the source pin630bb57a54b4ddda8338e0e529664436f982d94e→d648f25732dee1c974da62a94c46a642ad52cef8.
Not verified here
python tools/build_skill_index.py --source-revision d648f25732dee1c974da62a94c46a642ad52cef8 --checkandpython tools/build_skill_coverage.py --checkwere not run. Content hashes were not recomputed.- Did not confirm
d648f25732dee1c974da62a94c46a642ad52cef8is an ancestor of head8fd68009df7057d2be063e7bc0d2c9a5dc7511c0whose skill blobs match the recorded hashes. - Upstream NInfer at
1882c6adbc98043fcf89f3589aa5addb3742777awas not inspected. Tests were not executed.
|
Re-review of the new commits. I'm looking at Resolved since my last review
Still blocking
Not blocking for this PR, just a heads-up: #342 and #339 are still open and also edit Not verified by me: the upstream source pins in |
|
@zengchuang-hw @amy-why-3459, could you please review
|
zengchuang-hw
left a comment
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Incremental review cbbee590ec78c77ec15e4c169f80aeefa7ab671e..8fd68009df7057d2be063e7bc0d2c9a5dc7511c0. Reviewed head 8fd68009df7057d2be063e7bc0d2c9a5dc7511c0, PR base e16a5c2087fcd9cc4298794b4831e558eaefff46, and live main at 9f61a83f030bfa2ffe3b54eac1781593309d6380. Previous review · Integration review · Current request.
Resolved this round
- The current-head declaration passes and both Python full checks have completed. The earlier declaration failure is superseded by the successful current-head rerun. Policy · Ready CI.
- Both generation checks reproduce the checked-in projections from
d648f25732dee1c974da62a94c46a642ad52cef8. The 79 other owner cards only move the source revision. The merged metadata retains the prior Xe-Forge inputs, and the NInfer row is attributed to the existing benchmark owner. The trial merge into live main is conflict-free. Files changed. - Both NInfer reference files are present at the reviewed head, byte-identical to the earlier head, and match the hashes in the generated owner record. Reading the actual Git objects clears the truncated-patch concern. I verified the pinned source tree and the Git blob hash, SHA-256, size and line ranges for all 27 selected files. I cross-checked the same-physical-block fold and zero-row control, accepted versus adopted counters, receipt and device retirement boundaries, and the distinct historical measurement denominators. Prior non-voting review · Pinned source · Issue scope.
Before I can approve
No remaining blocking items for this revision.
Non-blocking observations
The pending sibling integrations can check their own combined source and projections against the predecessor they actually use.
Checks and limits
Locally ran both generation checks, the whitespace check and trial merges. Verified the current-head hosted Ready suite and the required policy status. Reviewed the source and unexecuted test definitions. Upstream C++/CUDA execution, model quality, measured performance and agent efficacy were outside this source-only slice.
Delegation record. Codex independently reviewed and submitted this record under direct delegation from zengchuang-hw through the connected zengchuang-hw GitHub account. The delegation covers independent review of PR #325 on the stated revision and allows review comments and formal Approve when the content and applicable checks satisfy the repository's requirements.
|
@amy-why-3459, sorry, I missed your re-review when I posted the review request. All three blockers are resolved at the unchanged head
$ python tools/build_skill_index.py --source-revision d648f25732dee1c974da62a94c46a642ad52cef8 --check
Skill index is current: 80 metadata records
$ python tools/build_skill_coverage.py --check
docs\skill-coverage.md matches its sourcesMain has since moved to |
Dong1017
left a comment
There was a problem hiding this comment.
PR review verdict — PASS (self-check; Approve blocked)
Reviewed base e16a5c2087fcd9cc4298794b4831e558eaefff46 and head 8fd68009df7057d2be063e7bc0d2c9a5dc7511c0.
Acting as InferRSI steward / Dong1017 stand-in. GitHub returned 422 Review Can not approve your own pull request, so this is COMMENTED only — no Approve/LGTM voting review from this account. Independent approvals remain required (one current Approve from zengchuang-hw).
Verified at this head:
- Additive NInfer merge on
inference-benchmark-designafter Xe-Forge (SKILL.md, metadata, both matrices); Xe-Forge content retained. ninfer-source.json(27 files) + contracts reference coherent as candidate/not_run; packaged sha256 match (ccda0fc6…,2b3de210…).- Upstream Neroued/ninfer
@1882c6ad…commit/tree and sampled blob SHA/size identities match via API. - Local: index
--check80 records fromd648f257…; coverage--checkmatches; ruff clean; focused skill/knowledge tests green. - Ready CI green; live
maine77f7f67merge-tree conflict-free with no intervening knowledge-path edits.
Not blockers here: no native NInfer execution; SkillEval 0 comparisons; #339/#342 remain separate.
The migrated snapshot named the pre-squash branch commit of #325. Point it at the squash commit on main, which has identical skill-library bytes, so the pin stays an ancestor after this change lands. Only _provenance.json changes. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Refs #91. Refs #177.
Problem and scope
Zero accepted drafts does not mean zero adopted input, and a commit receipt does not establish device retirement. This source-only amendment adds a NInfer-specific state-restoration and completion-validation reference to the existing
inference-benchmark-designowner, with an ordinary source ledger, a short entry pointer, incremental metadata and the two matrix mappings.The reference identifies coupled StateImage/Main/MTP history, the same-physical-block fold oracle, frontend-adopted versus accepted/committed counts, and publication/retirement observations. It preserves cancellation, zero/mixed/full-prefix controls and historical phase/corpus/wave denominator differences. Existing speculative-budget and Ouro recurrent-KV owners retain their separate responsibilities. No engine, controller, runtime, schema, registered skill or test implementation is added.
Fixed source and provenance
1882c6adbc98043fcf89f3589aa5addb3742777a.7f6aafedb5f20200def820cfe51ab81c09c20eeb, not the inspected source revision, and retain their quality/termination gaps.d356da0b8e4697cd4f584c837d8cbbacac22c13eis retained through main-preserving integration M7045a9eef66e310b4810fbb7d2d9d25ed1d14b24. Corrective source Mbad68ff5c1426b050df78ffbd106ac2e370ce186adds the existing benchmark owner ID to the Reference Matrix row, so the original parser associates the NInfer row with its owner. Both NInfer resources are unchanged since.Current-main integration (head
8fd68009df7057d2be063e7bc0d2c9a5dc7511c0)This follows the agreed order after #332, #322, #360 and #324 landed (main
e16a5c2087fcd9cc4298794b4831e558eaefff46), and addresses the cross-PR integration finding in the earlier review. The branch fast-forwards from the previous headcbbee590ec78c77ec15e4c169f80aeefa7ab671e; no history was rewritten.d648f25732dee1c974da62a94c46a642ad52cef8e16a5c208fd68009df7057d2be063e7bc0d2c9a5dc7511c0d648f257Xe-Forge (#322) already landed in the same
inference-benchmark-designowner, so three hand-written files needed a real merge. Every overlap is additive and keeps main's additions first, then this PR's:metadata.json: main's four tags, two sources, three failure modes, recommendation, limitation and two readsets stay; this PR appends its three tags, the NInfer source, two failure modes, applicability clauses (scope, input contract, produced artifact, refusal point), recommendation, limitation, two readsets and the two cross-owner references. The merge is by JSON field, and any concurrent edit that is not a pure append would have stopped it.docs/reference-matrix.mdanddocs/searchspace-matrix.md: this PR's NInfer row (1 line) and section (6 lines) are inserted directly after main's Xe-Forge row and section.SKILL.md: git merged cleanly; the 7-line NInfer paragraph sits after the existing treatment-delivery paragraph and before main's later additions.Relative to main
e16a5c20, the head changes exactly the six NInfer canonical paths plus 82 generated paths: 80 registered owner records, the generated knowledgeREADME.mdanddocs/skill-coverage.md. The two NInfer reference files are byte-identical to the previous head. The coverage view lists Xe-Forge and NInfer underinference-benchmark-design, and the DeepSeek, Infera and vLLM Ascend attributions are kept. Every owner record namesd648f257as its source commit.Commands run at this head, all exit 0:
git merge-tree --write-tree e16a5c20 8fd68009is conflict-free and produces exactly this head's tree0f2a22927af93a5b985f8d7c039f9e37260eb048.Focused local tests at this head (Python 3.13, imports bound to the checkout) passed with exit 0: 162 passed, 5 skipped across the index, coverage view, catalog, library/readers, knowledge contract, catalog CLI, Collect selection and knowledge CLI suites. The worktree stayed clean. Hosted Python 3.10/3.11 full and installed-package checks are the full-suite evidence for this head.
This integration has not had an independent review yet; formal review is requested on this head.
Earlier head evidence (
cbbee590ec78c77ec15e4c169f80aeefa7ab671e)The following results apply to the previous head over main
663f3b32and are not substituted for current-head checks.Both native check modes passed with 79 metadata records. Native owner/catalog reads verified all 12 source-file manifest hashes; Collect selected all 15 resources (189,640 bytes) with no omissions. Ready 41636127211 passed all six jobs: Python 3.10 and 3.11 full checks each reported 3,544 passed / 13 skipped / 1 warning / 284 subtests, and both fresh wheels loaded 127 records. Private independent gpt-6.1-sol / ultra review of that complete head returned LGTM (review-only). Codex review completed with no findings. A same-head SkillEval publication succeeded; an older concurrent publisher was refused by the anti-stale guard, and that history is separate from trial outcomes.
Limits
SkillEval selected 0 comparisons / 0 model calls, with the inherited executable-snapshot limitation retained. Knowledge remains candidate/not_run and efficacy remains not_evaluated. Upstream C++/CUDA/native assertions were read as definitions, not executed. No upstream build/test suite, model/device execution, service/quality experiment or paid trial was run. Software and source consistency do not establish native model parity, device performance or agent benefit. Pending Faster CosyVoice and Embodied increments are not imported; each later PR must preserve its canonical increment on the actual predecessor and regenerate projections from the combined source revision.
Knowledge authorship declaration