Repository navigation
Qualify RSI trials with explicit completeness and lifecycle evidence #340
Description
Current state: Contributor @amy-why-3459 has delivered merged #341, reviewed head 4cbe29b5374b9cf1282b1855e1631ec9e7b7e22e, for the bounded opt-in external-command trial checks and reusable workflow guidance described below. Selected literal-boolean adapter attestations are frozen in RunSpec and checked on both arms; these do not independently prove worker/device health, model quality or speedup. The PR was marked Ready at the preceding 8d01acd head; its description now also says ready for review. The earlier draft-stage Ready run passed its five applicable jobs and skipped KV qualification. All six jobs in the preceding 8d01acd0 Ready run passed. All six 89700595 Ready jobs, including KV qualification and both full suites, passed. All six jobs in the historical 1cdbb70 Ready run have passed by 09:35 UTC; this software result is distinct from review clearance and author admission. The preceding 8d01acd0 declaration check failed with 'Material AI author 1: model is unapproved or suspended'. The earlier declaration check failed on stale/invalid metadata. The prior PR declaration bound exact head 1cdbb70 and preserved both Codex and Cursor/Grok commit-attributed contributions. It recorded contributor-supplied Codex model gpt-6-astra with xhigh effort; exact historical Cursor model ID and effort remain unknown. That prior exception request covered all protected changed paths. The previous declaration check failed because independent owner exception approval was absent and both material authors' model identities were unapproved. After the declaration update, the then-current check, evaluated at head 1cdbb70 under main 9dded7a on 2026-11-05, still fails for the missing independent-owner exception approval and unapproved material AI author 2 (Cursor); it no longer reports author 1 as unapproved. This synchronization records the contributor's declaration, without independently authenticating historical configuration. Passing software CI does not resolve admission. The earlier scoped COMMENT review now reports one P2 evidence-presentation finding: failed/missing required trial checks suppress improvement_pct and yield INCONCLUSIVE, but the HTML/SVG consumer can still plot an apparent gain from raw medians. The reviewer requests suppression or unmistakable raw/unqualified labeling plus a report-level regression. The author's correction at 89700595 now reports a persisted trials_qualified flag, no derived gain chart for unqualified comparisons, raw-measurement labeling and report-level regressions. The six-file delta changes evaluation, reporting, chart handling, tests and documentation. The original reviewer's follow-up found a remaining pre-existing summary-chart.json path that bypassed the first repair. The author's second correction at 1cdbb70 now reports guarding both chart paths, retaining historical spec bytes without drawing them, and a regression for that exact case. The original reviewer's scoped recheck now clears this presentation finding at exact head 1cdbb70: the guard precedes both chart paths, historical JSON remains unchanged and raw medians stay labeled. It records all six Ready jobs passing and both full suites at 3,571 passed / 13 skipped, tested merge 38cf7fb with a tree matching the head. The same recheck confirms current-head author admission remains noncompliant under main 9dded7a; this is not whole-PR approval or acceptance. This synchronization reused that review without rerunning tests or duplicating it. The PR's earlier local review/evidence and disclosed failures remain separate. This is submitted implementation, not maintainer integration, goal acceptance or hardware/model qualification. The author has since merged main 6fefa010 and regenerated the index, producing preceding head 5e573b23. Its Ready CI succeeded; this historical branch-integration result is not approval of the later rebuilt head. The earlier declaration job failed with 'Missing, invalid or stale knowledge author declaration'. At 5e573b2 the PR then supplied a Codex-only declaration, and the then-current metadata check was green. However, Dong1017's review of that head and the independent follow-up both recorded DO NOT PASS pending reconciliation with the Cursor contribution recorded in commits 8cc3649 and 5e573b2, including protected generated paths. Green validation of that submitted metadata did not settle the provenance discrepancy. Both reviews retain clearance of the chart guard and report no new blocking runtime finding. The independent review's single focused Windows test failure also reproduces on unchanged base; it is not attributed to this PR. These are published review results, reused without a duplicate code review or test run.
The author has now rebuilt the branch, replacing the affected source/history and withdrawing the old exception request. The exact-head re-review at 4cbe29b5 against main e843b9d3 records no unresolved blocker and LGTM for the bounded software slice: the four identified Cursor/Grok commits are not ancestors, the replayed original Codex blobs match, the replacement chart guards cover both routes and stale SVG removal, and all 80 generated records pin the replacement source. It reports 57 focused tests plus synthetic route controls; all six Ready jobs pass, with both hosted Python suites at 3,772 passed / 13 skipped / 284 subtests. The trusted-main declaration assessment is compliant without an exception. This resolves the recorded discrepancy for this rebuilt head, without authenticating private execution settings or rewriting the prior-head history. That initial review was non-voting COMMENT feedback. Dong1017 subsequently submitted current-head APPROVED/LGTM, following explicit per-PR authorization recorded in the review. The independent incremental technical review now also has zengchuang-hw's formal APPROVED/LGTM. Both recorded approvals bind exact head 4cbe29b5374b9cf1282b1855e1631ec9e7b7e22e and begin with LGTM; both reviews report no remaining content blocker. PR #341 was merged on 2026-11-05 at 12:03:35 UTC as 01c73521ced6ca760cdfa42c9b937fe985a20d64. The checks above are the reviewed pre-merge evidence, not a newly verified post-merge check. The governing Issue remains open for requester goal acceptance. This synchronization reused the published reviews without another code review or test run and makes no hardware/model qualification claim.
Decision needed: The requester retains goal acceptance for the merged bounded software delivery. The old-head exception request has been withdrawn and the rebuilt head is recorded compliant without an exception; no old exception is presumed active.
Next action: Existing PR author @amy-why-3459 and the existing reviewers/integrator retain the post-merge verification and acceptance follow-up. Preserve historical review evidence, adapter-attestation limits and the absence of model/device qualification. This status update does not authorize new scope, execution, spending, merge or closure.
Goal and accepted scope
The contributor requested an InferRSI PR that incorporates workflow lessons from Omni model integration: reference parity, effective graph controls, streaming boundaries, and complete paired experiments with clean lifecycle evidence. Implementation and PR submission are delegated to Codex; merge and Issue closure remain with the owner/maintainers.
Bounded delivery
- Add opt-in trial qualification checks to the external-command harness, selected through CLI and frozen in RunSpec. Each baseline/candidate trial must provide literal-boolean evidence for the selected checks; missing, malformed, or false checks make the comparison INCONCLUSIVE and cannot support a speedup claim.
- Preserve the existing execution status, quality validation, source integrity, AB/BA order, and unconfigured command compatibility. Checks are adapter attestations backed by retained receipts, not automatic proof that devices or detached workers are healthy.
- Document reference numerical/RNG contracts, feedback-aware stage boundaries, matching full/stream reference paths, actual internal/outer graph switches, metadata-only/final-empty payload preflight, shared-device qualification, and full-comparison reruns after harness repair.
- Add CPU regression tests and update the discoverable RSI evidence workflow. Do not add campaign constants, default accelerator jobs, model dependencies, or a new CI lane.
Acceptance
- Missing/false/non-boolean checks on either arm cannot produce PASS or a qualified improvement.
- Complete checked trials can pass existing gates; commands without selected checks remain compatible.
- CLI -> RunSpec -> persisted trial evidence -> evaluation is exercised with real synthetic commands.
- Documentation distinguishes framework tests, adapter claims, and actual hardware/model qualification.
Related source: vllm-project/vllm-omni#8972 (workflow updates). This task makes no model-quality or hardware-performance claim.
Activity
agent-ci-infra-bot commentedon Nov 4, 2026
Intent: add opt-in trial-qualification checks to the external-command harness — CLI-selected, frozen in RunSpec, persisted per baseline/candidate trial — so missing, malformed or false attestations make the comparison INCONCLUSIVE instead of supporting a speedup claim, with CPU tests and RSI evidence documentation. The named code surfaces exist, but the check set, CLI/evidence schema and the repository-required status/decision record are missing. Verdict: needs information / human decision required; do not start development yet.
Labels: kind:task, area:workbench, area:knowledge (bounded harness delivery plus the RSI evidence workflow; no priority because the Issue has no status:proposed/status:accepted label or recorded owner decision).
F1 — needs information — the selected check set and per-trial evidence contract are undefined
src/inferrsi/runspec.py:9-35(no check field),src/inferrsi/arguments.py:47-104(no CLI selector),src/inferrsi/harness/scenarios/external.py:12-32(writes onlyexecution.jsonand the metric),src/inferrsi/harness/evaluation.py:44-66(gates only on trial status, metric, validation and integrity).truevs"true"vs1). The external harness has no receipt contract; the only receipt mechanism in the tree is Skill Eval'sreceipt.json(docs/skill-eval-comparison.md:217), a different lane. The linked vllm-omni PR is documentation for another repository, not a schema authority.run.lock.json(src/inferrsi/harness/runner.py:34) and trial evidence; regression tests cannot assert the intended behavior, and reviewers cannot check the acceptance bullets.F2 — needs information / human decision — the required Issue status block, labels and a recorded acceptance are missing
.github/ISSUE_TEMPLATE/scoped-task.md:17-19,docs/agent-collaboration.md:191-194,212-216.**Current state:**,**Decision needed:**,**Next action:**block, nokind:*orarea:*labels, and the Issue has zero comments. It claims "accepted scope" and delegates implementation to Codex, butstatus:acceptedrequires the decision owner to record that decision in a comment (docs/agent-collaboration.md:194), and delegation must be recorded or linked (docs/agent-collaboration.md:15).kind:task,area:workbench,area:knowledge).F3 — needs information / human decision — one Issue mixes harness code with a broad docs/skill delivery, including protected knowledge
docs/knowledge-model-policy.md:31-43,src/inferrsi/data/skill_library/workflows/rsi-evidence/SKILL.md:1,docs/agent-collaboration.md:97.rsi-evidenceskill, which lives undersrc/inferrsi/data/skill_library/**— protected scope requiring an approved model atxhigh/maxand aknowledge-model-policyPR declaration. The Issue delegates to Codex without naming a model or effort, and it does not distinguish the skill from ordinary docs such asdocs/contributing.mdordocs/quickstart.md. The docs bullets (reference numerical/RNG contracts, stage boundaries, graph switches, preflight, shared-device qualification, reruns) are a separate delivery from the harness check.knowledge-model-policystatus, and the mixed scope is hard to review against the acceptance bullets.rsi-evidenceskill explicitly in or out of scope.F4 — scope risk — compatibility acceptance does not cover persisted RunSpec readers and shared-runner adapters
docs/architecture.md:4(RunSpec and command contracts "remain stable"),src/inferrsi/harness/runner.py:34,tools/qualify_kv_reclamation.py:165-181,examples/qwen21_turbo_validation_adapter.py:21.run.lock.jsonand of the shared runner used by the synthetic KV qualifier and legacy validation adapters. Acceptance only says "commands without selected checks remain compatible"; it does not require absent-field/default-off behavior or consumer regression coverage.tools/qualify_kv_reclamation(or an equivalent unconfigured adapter) through the new evaluation.F5 — related work — PR #202 already tightened the same harness verdict/state path, and Issue #55 may overlap
src/inferrsi/harness/evaluation.py,src/inferrsi/harness/runner.py,src/inferrsi/harness/scenarios/external.py.tests/test_runtime.py) already made run state and comparison verdicts match retained evidence. Issue Add Qwen3.8 FP8 correctness and service qualification workflows #55 (open,area:workbench) adds a correctness objective and qualification workflows to the shared evaluator. The Issue cites neither, so the new checks could duplicate or conflict with that verdict contract.Reviewed commit: 663f3b3
Coverage: Checkout: 2645 files at 663f3b3 in a git worktree with the repository's history
Dong1017 commentedon Nov 5, 2026
Closing: the bounded delivery is merged. This Issue reserved merge and closure for the owner/maintainers rather than the contributor; recording that acceptance here, verified read-only against
main.PR #341 merged at 2026-11-05 12:03:35 UTC as
01c73521, from reviewed head4cbe29b5with two approvals bound to that exact head.Each bounded-delivery bullet, against
mainINCONCLUSIVEand cannot support a speedup claimrequired_trial_checksis a frozen tuple onRunSpecwith validation (src/inferrsi/runspec.py:36,49,80-85)skill_library/workflows/rsi-evidence/with its catalog recordThe final review recorded 57 focused tests plus synthetic route controls, all six Ready jobs passing, and both hosted Python suites at 3,772 passed / 13 skipped / 284 subtests.
The provenance question was resolved by rebuilding, and that matters
This PR spent most of its life blocked on author admission, not on code. Declarations failed repeatedly for unapproved or unknown material AI authors, and a requested owner exception covering 87 protected paths was pending. The contributor withdrew that exception request and rebuilt the branch so the four Cursor/Grok commits are no longer ancestors, the replayed Codex blobs match, and all 80 generated records pin the replacement source. The trusted-main declaration assessment is then compliant without an exception.
Recording this because it is the better outcome: the admission requirement was met by removing the unattributable history, not by granting an exception around it.
One P2 evidence-presentation finding was also real and fixed: an unqualified comparison could still be plotted as an apparent gain from raw medians. The repair guards both chart paths, keeps historical spec bytes without drawing them, and carries a regression for that exact case. The reviewer cleared it at the exact head.
What this closure does not claim
The checks are adapter attestations backed by retained receipts. They do not independently prove that detached workers exited cleanly, that devices were healthy, or that the intended graph controls actually ran — the harness does not inspect those. Nothing here is a speedup, quality or hardware result, and applying these checks to a real campaign still needs frozen adapter validators and retained request/worker/graph receipts.
The merge evidence above is pre-merge review evidence; no new post-merge verification was run for this closure.
Reopen if: the qualification path is applied to a real campaign and the attestations prove unable to carry the evidence they claim.
Closing as completed.