Repository navigation
Conversation
Describe observation-to-action timing, replay denominators, precision and per-task quality evidence using the existing benchmark owner. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
Build the native catalog and coverage from source revision d5fc212. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author)
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 4c778ff95e77dfdcdd869f2e8a17d18d77cc7122 --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/embodied-cpp-boundary-and-quality.md","src/inferrsi/data/skill_library/methods/inference-benchmark-design/references/embodied-cpp-source.json"],"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/_provenance.json","source_revision":"4c778ff95e77dfdcdd869f2e8a17d18d77cc7122","status":"ready"}Provider/settings: Uncovered / unsupported paths (9)
|
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Source-only LGTM for the bounded #91 VLA benchmark-guidance amendment at 0f4433b7c9bbc1512a4ef5526f087c2a9730d239, against main 663f3b327998526a13aa31e72ad729025c18983c. No blocking change found in this scope.
Existing review disposition: refreshed the conversation, submitted reviews and inline threads before publication. There are no existing submitted reviews or inline threads to resolve; the automated current-head review is complete with no findings. The SkillEval evidence limitation below remains explicit rather than being counted as a successful evaluation.
Checks performed:
- Inspected all 87 changed paths by their source/generated roles. All 78 unrelated generated records change only source provenance. Source commit
d5fc2121is reachable and its canonical bytes are unchanged at this head; all 12 source-file SHA-256 values in the affected projection match fetched current-head bytes, and projected sidecar fields match their source. - Matched the 17 recorded upstream paths, byte counts and Git blob identities against the complete 213-entry pinned tree. Re-read the relevant timing/pair gate, queue/replay, profiler, aggregation, server response, quality and attribution sources, plus paper v3 §4. Recomputed the public 100+100 latency arrays and memory sample summaries. The guide preserves complete-request versus generated-action timing, per-task loss, precision differences, whole-device memory and missing measured-source/raw-episode evidence; it does not attribute the later SmolVLA experiment to paper v3.
- The existing owner stays
candidate/not_run, with no hardware prerequisite or new SearchSpace mechanism. Public sample arithmetic is not a new device measurement or an independent recount of unavailable episode logs.
Current hosted evidence: all six Ready jobs and the declaration check pass. The inspected Python 3.11 log reports Ruff success, 3,544 passed / 13 skipped / 284 subtests; the isolated wheel check reads 127 source-grounded records. Tested merge 954ff5c6 has zero file differences from this head. I did not rerun repository tests or execute upstream code.
The SkillEval plan selected zero comparisons and was not executed. Its unsupported executable snapshot entry is already present with the identical mode/blob on main; this review neither relaxes that safety boundary nor treats unavailable/unknown budget or evaluation as a pass. No runtime, robot, device, quality-preservation, performance or Agent-efficacy result is established. #339's overlapping pending amendment was not combined or tested here. This COMMENT is not formal APPROVE, merge authorization, or acceptance/closure of the broader #91/#177 goals.
Dong1017
added
the
area:knowledge
ChatGPT Codex Connector
|
Review at head
Not verified by me: the upstream source pins/byte hashes in |
|
Re-review at head No new commits since the prior short-SHA note. Isolation content (Embodied.cpp VLA reference) still looks coherent, but blockers remain:
Not verified by me: upstream source pins/byte hashes in Reviewed commit: |
Merge main after the earlier source PRs and the shared source snapshot landed. Where this PR and main extend the same files, keep main's additions first, then the Embodied C++ additions. 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 main's writers on the committed combined source. Only the edited owner's record, the shared source snapshot and the coverage view change. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base bd61d20 and head f68c9f9 (full diff; this seed has no previous head).
Bounded VLA observation-to-action guidance on inference-benchmark-design: new readset and procedure, skill contract and failure modes, reference and search-space rows, and the generated coverage and knowledge-pack projection. The seeded prior note is Dong1017's source-only LGTM at 0f4433b7c9bbc1512a4ef5526f087c2a9730d239 (different base/head), not an open Dong-bot blocker.
Verified in the diff:
- The 17 Embodied.cpp paths in
metadata.jsonmatch the 17filesentries inreferences/embodied-cpp-source.json, pinned at156bed151826780e2ce0132f8d0e3540889ae8cd. - Script SHA-256
2a7e94a4df662d2da41169cc2345b75e683c82e082fe590eba713302255e1a7b, artifact SHA-256db53513cfdc52a03c3a4f2f1a2bfbab7a1eed45a9995873ac3bf34b7e9d7019d, and LICENSE SHA-25688993458addec64e6922f552eb27ddd5f775d911db56205f167c885800011bf7agree across the procedure, the source record, and the provenancesourcesentry. - Recorded summary arithmetic matches the stored fields:
308.0299474900005 / 50is the C++ per-generated-action mean, the request ratio is0.6276648803191476, and1029/1671is the sampled-memory ratio0.6157989228007181. Success counts 166/200 and 172/200, with task 7 at 9/20 versus 20/20, stay attached to the author tables. Per-generated-action cost is amortization; control-rate and quality-equivalence conclusions stay withheld. Paper reuse stays section 4/Table 3, with the wider inspection recorded on the readset. SKILL.md,metadata.json, and both applicability copies in the knowledge pack carry the same VLA scope, five failure modes, refusal clause, recommendation, and Embodied.cpp limitation. Coverage moves from 7 to 8 advice and from 13 to 15 readsets, matching the added recommendation and two provenance readsets.empirical_validationstaysnot_run. Search-space hardware stays source context; the existing N/A reason is unchanged.
Not verified here:
- Upstream blobs, the 213-entry tree, arXiv v3, and the raw latency arrays were not re-fetched, so those blob identities and the sample recomputation were not independently reproduced.
python tools/build_skill_coverage.py --checkwas not run, and CI was not inspected.- Knowledge-pack
source_revision4c778ff95e77dfdcdd869f2e8a17d18d77cc7122was not checked for reachability or byte identity against this head. The registry hash is unchanged, andskill-index.jsonis not in the diff.
No blocking finding in this scope. Author-reported sample arithmetic stays a published-array audit, separate from a local device measurement or efficacy result.
amy-why-3459
left a comment
amy-why-3459
left a comment
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base bd61d20c52b4d772a3ea282ae9cb3c0fd50075df (current main) and head f68c9f948875693bddb1da84699cbbcb7849beec. This is incremental from my last review at 0f4433b7c9bbc1512a4ef5526f087c2a9730d239, covering the main merge 4c778ff9 and the regeneration in f68c9f94.
- Earlier blockers are resolved. The branch is up to date with main (0 behind), and all the sibling knowledge PRs I flagged (#339, #332, #325, #324, #322) have merged. Against main, the PR is now 9 files, all for the Embodied.cpp VLA reference.
- The content merge kept the siblings' content. In
inference-benchmark-design/metadata.json, every changed line is an existing entry extended with the VLA clause, and the NInfer, Faster CosyVoice and Xe-Forge content from the merged PRs is intact. InSKILL.md, only thedescriptionwas reworded (adding VLA), plus a new VLA paragraph. - Generated records match the source.
_provenance.jsonpins4c778ff9. All 17source_fileshashes inknowledge/common/skills/inference-benchmark-design.jsonmatch the file bytes at both4c778ff9and the head.docs/skill-coverage.mdhas a new digest, and the matrix rows were added in place. - CI is green on this head.
full-check(3.10 and 3.11),installed-package(3.10 and 3.11),pr-references,planandpublishpass. One earlierdeclarationsrun failed, and a later run on the same head passed.
Still unverified by me: the upstream source pins and byte hashes in embodied-cpp-source.json, which I didn't check against the upstream repository. Static review only, no local test run.
VLA benchmark reports can mistake latency amortized over a generated action chunk for real control cadence, and aggregate success can hide task-specific quality losses. This change adds source-bound observation-to-action, replay-denominator, precision and per-task quality guidance to the existing
inference-benchmark-designowner.Sources are Embodied.cpp at 156bed15, licensed Apache-2.0, and the companion paper v3, CC BY 4.0. The source record identifies 17 inspected files and their exact versions. The later SmolVLA experiment is not presented as a paper-v3 result.
The reference preserves the different measured-script hashes, incomplete dirty patch and absent raw episode records, as well as the reported precision and whole-device memory boundaries. Public sample arithmetic is identified separately from author-reported quality results. The registered owner remains candidate /
not_run, its hardware field remains absent, and its original SearchSpace resource exclusion remains intact.Current-main integration (head
f68c9f948875693bddb1da84699cbbcb7849beec)This follows the agreed order after #332, #322, #360, #324, #325, #359 and #339 landed (main
bd61d20c52b4d772a3ea282ae9cb3c0fd50075df), and addresses the cross-PR integration blockers in the earlier review. The branch fast-forwards from the previous head0f4433b7c9bbc1512a4ef5526f087c2a9730d239; no history was rewritten.4c778ff95e77dfdcdd869f2e8a17d18d77cc7122bd61d20cf68c9f948875693bddb1da84699cbbcb7849beec4c778ff9Xe-Forge, NInfer and Faster CosyVoice already extend
inference-benchmark-design, so four hand-written files needed a real merge. Main's additions come first, then this PR's:metadata.json: merged by JSON field. Main's 7 tags, 4 sources, 5 failure modes, 2 recommendations, 3 limitations, 6 readsets and the three applicability clauses stay; this PR appends its 4 tags, 2 sources, 5 failure modes, 1 recommendation, 1 limitation, 2 readsets and its clause on each of scope, input contract, produced artifact and refusal point. The merge stops on any concurrent edit that is not a pure append.SKILL.md: the 7-line observation-to-action paragraph is added after the Faster CosyVoice paragraph. This PR also replaces the one-linedescription; main had not changed that line, so the replacement applies unchanged.docs/reference-matrix.md: the Embodied row (1 line) follows main's rows.docs/searchspace-matrix.md: the 13-line Embodied section follows main's sections at the end of the file.The two new reference files are byte-identical to the previous head. With the shared source snapshot from #359, regeneration changed 3 generated files: the
inference-benchmark-designrecord,_provenance.json(now pinned to4c778ff9) anddocs/skill-coverage.md. Relative to main, the head changes 9 paths: the 6 canonical paths and these 3.Commands run at this head, all exit 0:
git merge-tree --write-tree bd61d20c f68c9f94is conflict-free and produces exactly this head's tree0c01f6c263c2d1c59c677eb207136efcb936d0e0.A local focused test run was not repeated at this head: the host stopped the same run for low memory on the previous PR. Hosted Python 3.10/3.11 full and installed-package checks are the test evidence for this head.
Earlier head evidence (
0f4433b7c9bbc1512a4ef5526f087c2a9730d239)The following results apply to the previous head over main
663f3b32and are not substituted for current-head checks.Canonical source M:
d5fc2121d6d1628111403ba70ec3b70638ab8674. Native generated G and head:0f4433b7c9bbc1512a4ef5526f087c2a9730d239.Native index and coverage write/check modes passed from the actual M. At that head, complete checkout and source-package resource reads matched the committed bytes; the generated index and coverage were verified against the actual sources. An independent Sol/ultra review of that complete head found no blocking source or generated-projection issue. Ready CI passed against a tested merge whose tree equals that head's tree. Python 3.10 and 3.11 each reported 3,544 passed, 13 skipped, 1 warning and 284 subtests; both isolated installed-wheel checks read 127 source-grounded records.
Limits
The source-derived candidate does not establish upstream runtime, model, robot, device, quality, performance or agent-efficacy results. The snapshot pin
4c778ff9is a branch commit; after a squash merge it is not an ancestor of main, and the next knowledge PR re-pins when it regenerates.Refs #91. Refs #177.
Knowledge authorship declaration