Repository navigation
knowledge: distinguish Xe-Forge trial origin and timing - #322
Conversation
Bind saved kernel trials to result origin and comparator identity. Keep host gates, selection parity, caller defaults and oracle limits explicit. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
Require result-origin and evaluator receipts before treating a stored pass as observed acceptance. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex (gpt-6.1-sol, reasoning_effort=max)
Regenerate coverage and the catalog from the committed Xe-Forge source amendment. 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 5898e717015272257fdd14e06d8ae6b542d2c3ce --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/xeforge-source.json","src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/xeforge-trial-evidence.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":"5898e717015272257fdd14e06d8ae6b542d2c3ce","status":"ready"}Provider/settings: Uncovered / unsupported paths (88)
|
Dong1017
left a comment
There was a problem hiding this comment.
Source review found no actionable defect in the Xe-Forge amendment at 76b680a6. Skill Eval still has no executed comparison; this is a scoped source/software COMMENT, not an unconditional merge-readiness conclusion.
- Verified source claims: all 18 pinned source files match the recorded byte counts, SHA-256 and Git blob identities. The inspected host/manual/Claude paths support the distinctions between recorded origin, comparison validity, conditional ratio updates, measured-only selection, delivered bytes and baseline-oracle limits.
- Software evidence: current Ready and per-head declaration checks pass. The tested synthetic merge has exactly the head tree. The canonical source-to-head delta contains only 81 projection files; the other 78 cards change only their source URL/commit headers.
- Remaining limits: the Skill Eval plan selects zero comparisons and retains the executable-snapshot refusal. Its nonpaid index-check success is author-reported and checks projection consistency. Source fidelity is verified within the inspected paths; skill benefit, native execution and model/service acceptance remain unverified.
- Next: the existing PR owner, Dong1017, retains integration. Use this bounded source/software disposition alongside the remaining evaluation limits when making the integration decision; no new implementation work is assigned.
Evidence and review scope
- Reviewed head:
76b680a6a2be842e7999ffaf4a1a31c76c3dfac2. - API base / observed main:
2baebf6ebb815ebf07d21c41e3b3754858c0798e. - Canonical source:
4c0888c1ec85d02f4b5d753e2389698652baf8b4; projection-only delta. - Xe-Forge pin:
6cd3ee98dfe32746b345a6abe521fa4cb9bdc187. I reread the recorded source identities/ranges and traced the amendment's claims through the pinned trial manager, benchmark, external host, CLI, trial/finalization and Claude-engine paths. I checked relevant paper v1 context and claim boundaries; the retained HTML digest, unused graphics and empirical results were not independently validated. - Ready run passed all six jobs. I inspected the Python 3.11 full-check log: Ruff passed; 3,389 tests passed, 13 skipped, 284 subtests passed. Checkout
d58d053437ab62452fe6a05f6233d369a10de9edhas main/head parents and the same Git tree as head:8062924cf9e8fa2e9b98ddb1233b37d1f2b6a9d3. - Current declarations passed. Skill Eval remains PLANNED with execution skipped. I assessed the nonpaid handoff under the current contract: the source review supports source-fidelity claims, while a path mapping or index check does not establish task benefit.
- Independent maintenance review; I did not implement this PR. This run performed static reads, hashing and existing-log inspection only. No local suite, upstream tests, model/device execution or paid trial was run. Candidate/
not_runstatus and efficacy limits remain applicable. This comment does not accept the governing Issues.
Preserve the existing Xe-Forge source reference and main applicability, resource-layer and matrix 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)
Rebuild the native catalog and resource-layer coverage from the actual combined canonical source revision. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
|
The PR adds a source readset and an interpretation reference for IntelLabs/Xe-Forge to the Reviewed commit: ed8b306 |
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)
Generate native projections from actual corrected canonical source bb47209. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
|
Head Reviewed commit: c624c4c |
|
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 (Xe-Forge reference + SKILL.md pointer) still looks coherent, but blockers remain:
Not verified by me: upstream source pins in Reviewed commit: |
Merge main after the Infera source landed, preserving the Xe-Forge references, owner metadata 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 c38bb4e97db3ea4be4059b80376e0925e48c0df4 and head 8b4d05252a048a2672ede73170e281220a4b48dc (#322). Seeded diff mode is full and previousHead is empty, so this re-review is against main and is separate from the amendment previously inspected at 76b680a6.
Blocker — the source pin and trial note are not fully reviewable on this head.
src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/xeforge-source.jsonis a 464-line addition (index 000000000..9aa863ce0). The seededpr/diff.patchstops inside the first file entry'srights_note(CONTRIBUTING.md, git blob3b253e011739bfd1437876e533e694a7a9991e3b). The other 17 pinned paths and their SHA-256 values, byte counts, and exclusion notes are absent from this review diff.references/xeforge-trial-evidence.mdis the noteSKILL.mdnow points to (new paragraph at the insertion around line 188), and it is listed frommetadata.jsonprovenance_readsets,docs/reference-matrix.md,docs/searchspace-matrix.md, andknowledge/common/skills/inference-benchmark-design.json(recorded content SHA-256802c4dfe83983cbd6740231d5d0e2b459792b685de8dfbefb6dc51a912e23604). Its body is absent from the seeded diff.
Trigger: the matrices and metadata assert an 18-file pin at 6cd3ee98dfe32746b345a6abe521fa4cb9bdc187, named-verdict update limits, measured-only versus manual delivery, and baseline-oracle limits. Those assertions need the note and the rest of the pin on head 8b4d052.
Expected correction: include the complete xeforge-source.json and xeforge-trial-evidence.md in the reviewable diff for this head. I will re-read after that push. This comment does not approve and does not change scope.
Visible on this diff, still consistent where the text is present
SKILL.md,metadata.json, both matrices, and theinference-benchmark-designknowledge card name the same 18 Xe-Forge paths, paperarxiv:2605.26118v1,empirical_validation: not_run, and efficacy not_evaluated. New failure modes keep tool-origin, a displayed/manual best, ratio omission, and baseline agreement distinct from a measured gain.docs/skill-coverage.mdmovesinference-benchmark-designto 6 advice and 9 readsets. Input digest issha256:19d575629009048019f2fd2dfcf1f4c4afd1d529fa4cfc27e65dfb3103768f6f. Other knowledge cards in the seeded diff change the source snapshot URL/commit from8d40de5673666430573a5d1c1aa3f17e906a1be3to5898e717015272257fdd14e06d8ae6b542d2c3ce.- Prior comment on
76b680a6recorded no actionable defect there. Skill Eval still selects zero comparisons on this head (not_run/ not_evaluated). That limit stays outside this source-record slice once the two files above are readable.
Not verified here: Xe-Forge blob/SHA-256/byte recomputation, the xeforge-trial-evidence.md text, build_skill_index.py --check, build_skill_coverage.py --check, CI, and any executed Skill Eval comparison.
amy-why-3459
left a comment
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base c38bb4e97db3ea4be4059b80376e0925e48c0df4 and head 8b4d05252a048a2672ede73170e281220a4b48dc; incremental range since my last review c624c4c1c0e6631e026a2c2f7facef51719736ad...8b4d05252a048a2672ede73170e281220a4b48dc (merge 5898e717 + regeneration 8b4d0525).
Earlier blockers resolved:
- Dirty vs main: the branch now merges current main
c38bb4e9(after #332 landed) and GitHub reports it mergeable. Main's only later commit952e5107touchesdocs/kv-reclamation-qualification.mdand does not overlap this PR. - Cross-PR integration: rebased/merged in the stated order after #332, projections regenerated. The follow-up rebase burden now falls on the remaining sibling knowledge PRs (#342, #339, #325, #324) once this lands.
Verified:
- The four owner-local files (
SKILL.md,metadata.json,references/xeforge-source.json,references/xeforge-trial-evidence.md) are unchanged between the previous and current head; the net diff vs main is the 6 canonical paths plus 82 generated paths (80 records at 2+/2− source-snapshot bumps, knowledgeREADME.md,docs/skill-coverage.md), and theinference-benchmark-designcard. - Recomputed SHA-256 at head for
SKILL.md(a7ea13bc…),metadata.json(cf42c693…),xeforge-source.json(6c21791b…) andxeforge-trial-evidence.md(802c4dfe…); all match thesource_filesentries inknowledge/common/skills/inference-benchmark-design.json. - Matrix rows (reference + searchspace) are additive only and consistent with the card (18 pinned Xe-Forge paths at
6cd3ee98, candidate/not_run, efficacy not_evaluated). - Hosted checks on this head are green, including
full-check (3.10/3.11),installed-package (3.10/3.11),declarationsandpr-references.
Not verified by me: upstream Xe-Forge blob/byte pins inside xeforge-source.json, and any executed Skill Eval comparison (still zero selected).
|
@zengchuang-hw, could you please review
|
zengchuang-hw
left a comment
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Review range c624c4c → 8b4d052 for the earlier integration review. Current head 8b4d05252a048a2672ede73170e281220a4b48dc, PR base c38bb4e97db3ea4be4059b80376e0925e48c0df4, targeting main. I verified the current integration and the complete source amendment in response to the review request.
本轮已解决 · Resolved
- The complete readset and trial note are readable from the exact Git revision. Both and the two owner files match the preceding head byte for byte and match their generated card hashes. I fetched all 18 pinned files and checked their tree entries, Git blob IDs, byte lengths, SHA-256 and cited line bounds. The relevant source predicates support the note's distinctions among tool origin, conditional ratio clearing, candidate ranking and baseline-only oracle evidence. The Cursor comment identified a review input truncation. The full Git objects close that evidence gap. Upstream pin · Paper v1
- The six canonical paths contain the bounded Xe-Forge methodology amendment. Of the 80 regenerated records, 79 differ only in their shared source pin. The two matrices retain the existing source rows. Both local generator check modes passed from the combined source revision
5898e717. The trial merge with current main952e510is clean and adds only the separate KV documentation page relative to the reviewed head. Current diff · Governing backlog
批准前必做 · Before I can approve
No outstanding items.
非阻塞问题或建议 · Non-blocking
Later sibling knowledge PRs need their own reconciliation and regenerated outputs after this PR lands.
I checked the exact-head Ready workflow and knowledge-model-policy. Both passed. The local checks were the two generation checks and diff whitespace. Hosted checks provide the full-suite and wheel evidence. The scope is source-derived guidance. No upstream runtime or SkillEval model comparison was executed in this review.
Codex performed this review under direct delegation from zengchuang-hw, using that connected GitHub account. The delegation covers independent review and feedback on this PR and formal approval of its verified head when the applicable checks pass.
Refs #91. Refs #177.
Problem and scope
Xe-Forge's tool-recorded origin, valid timing, comparison ordering and operation success describe different facts. This source-only amendment adds one reference and an ordinary source-readset JSON to the existing
inference-benchmark-designowner, with an entry pointer, incremental metadata and both matrix mappings.The guidance distinguishes caller-reported correctness from an observed oracle result, first-pass cohort pinning from complete measurement validity, and unresolved-comparison parity from a measured gain. It preserves conditional stale-ratio clearing, manual versus strict selection defaults, saved-versus-delivered byte identity and baseline-only oracle limits.
Fixed sources
6cd3ee98dfe32746b345a6abe521fa4cb9bdc187, Apache-2.0. The readset records exact inspected ranges for 18 source, rights and test-definition files.Original Xe-Forge canonical source
4c0888c1ec85d02f4b5d753e2389698652baf8b4is retained through integration Maed1f791f5aac75d930d14ae7feef743b32b4b57over main1fca842ad692bafc1de5e005d10fd755d7e6534f. Corrective source Mbb4720992ac2a515adc09b645e1e3cfda47e9d69names the existing benchmark owner in the Reference Matrix row, so the original parser attributes that source row to its owner. This methodology owner remains N/A for hardware-resource coverage.Current-main integration (head
8b4d05252a048a2672ede73170e281220a4b48dc)This follows the agreed order after #332 landed as
c38bb4e97db3ea4be4059b80376e0925e48c0df4, and addresses the cross-PR integration finding in the earlier review. The branch fast-forwards from the previous headc624c4c1c0e6631e026a2c2f7facef51719736ad; no history was rewritten.5898e717015272257fdd14e06d8ae6b542d2c3cec38bb4e9vllm-ascendand all other changes since1fca842a. Only generated projections conflicted; they kept main's bytes until regeneration8b4d05252a048a2672ede73170e281220a4b48dc5898e717Relative to main
c38bb4e9, the head changes exactly the six Xe-Forge canonical paths plus 82 generated paths: 80 registered owner records, the generated knowledgeREADME.mdanddocs/skill-coverage.md. The four owner-local files are byte-identical to the previous head. Both matrices auto-merged; their delta from main equals this PR's original delta, and the Infera rows from #332 are kept. Every owner record names5898e717as its source commit.Commands run at this head, all exit 0:
git merge-tree --write-tree c38bb4e9 8b4d0525is conflict-free and produces exactly this head's tree771e72652a514945cec417d5d61bbb4a306152fc. Main has since advanced to952e510760eec528e338d474ea08acc737829793(#369), which changes onlydocs/kv-reclamation-qualification.md; the trial merge with that main is also conflict-free and differs from this head only in that page. The coverage view attributes Xe-Forge toinference-benchmark-designand still maps both Vidur and Infera tocalibrate-serving-simulation.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 (
c624c4c1c0e6631e026a2c2f7facef51719736ad)The following results apply to the previous head over main
663f3b32and are not substituted for current-head checks.Both original native write modes passed from the corrected M, and both check modes passed at that head with 79 metadata records. Owner/catalog/checkout/source-package and revision-bound resource reads passed. Whole-resource Collect selected 12 resources (154,669 bytes) with no omissions.
Ready 41632654789 passed all six jobs. Python 3.10 and 3.11 full checks each reported 3,544 passed / 13 skipped / 1 warning / 284 subtests, plus Ruff. Both fresh isolated noneditable wheel installations loaded 127 source-grounded records. Private independent gpt-6.1-sol / ultra review of that complete head returned LGTM (review-only). Codex review completed with no findings.
Limits
SkillEval has zero comparisons / zero model calls. Knowledge remains candidate/not_run and efficacy remains not_evaluated. No upstream suite, model/device execution, kernel benchmark, quality/service experiment or paid trial was run. Pending DeepSeek, NInfer, 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