Repository navigation
Conversation
Signed-off-by: Satya Srinivas Anandh <satyasrinivasanandh@gmail.com>
Signed-off-by: Satya Srinivas Anandh <satyasrinivasanandh@gmail.com>
panthera-momagdii
left a comment
There was a problem hiding this comment.
Initial, scoped review at 8440f4b818f4bd0e6bbbbcc1538eee0555e0012a. I inspected the checker and semantic-gate implementation, native corruption/read-only tests, the C10 workflow, and the recorded limitations. This is not a full audit of every changed file or a local execution of the suites.
I independently checked the hosted results: C10 reference run 38861626485 and its nonempty-suite step succeeded; ROS 2 Jazzy run 38861626521 and its quality/build verification step succeeded; Repository quality run 38861627006 also succeeded. These are useful implementation results, not proof of a real recorded episode or production acceptance.
The distinction between REJECT and NOT_VERIFIED, explicit policy inputs, preservation of the C9 contract, and keeping synthetic data out of training-ready acceptance are the right direction. For the placement discussion, my recommendation is to keep this a conformance-reference consumer under tools/ and tests/ while runtime ownership is agreed separately. This comment does not approve a runtime home, numeric policy, release pin or source profile.
Please keep the PR draft. Next steps:
- Record the work-package owner's explicit adopt/adapt/reject decision and the unresolved owner decisions in one short checklist. Do not fill missing release/timing/action/calibration policies with baseline defaults.
- Attach one accessible, reproducible C10 CLI report with the exact command, checker SHA and dataset revision. A synthetic report is useful for reviewing the reporting path, but must remain labelled synthetic and NOT_VERIFIED. The next real-episode acceptance report must include its matching native artifacts and C9 sidecar.
- Consider retaining that report and the native-suite test output as workflow artifacts, including on failure; the C10 workflow currently runs the suite but has no upload step. Local ignored evidence paths are not a handoff other reviewers can retrieve.
No merge approval or completed-C10 claim is given here. Review assisted by ChatGPT; no local ROS, native-video, dataset or hardware run was performed in this review.
Signed-off-by: Satya Srinivas Anandh <satyasrinivasanandh@gmail.com>
panthera-momagdii
left a comment
There was a problem hiding this comment.
Reviewed the evidence-handoff follow-up from 8440f4b to 34fd6de3fa9735ee550c2da4da28b48e31bc1f9d.
Review basis: openAMRobot/openamrobot-interfaces, branch c10-dataset-quality-gate, head 34fd6de, base 87ab8a9. Expected outcome: close the evidence-access follow-up while keeping C10 acceptance and owner adoption explicit.
The five-file follow-up addresses the requested report access and retained test output. The helper preserves the checker's raw NOT_VERIFIED result and exit code 2, records the reviewed commit and available inputs, and retains pipeline failures rather than turning a successful tee into a passing test. The core checker, C9 contract, and dependencies are unchanged in this delta.
I inspected the hosted C10 job: 63 core tests, 31 native tests, and 23 evidence tests completed successfully, including the expected arithmetic-revert negative control. The evidence artifact is available at run 38938591049, artifact 12062027077. The separate Jazzy verification run also passed, including its expected consumer failure after the revert. I verified the job output and artifact availability; I did not independently unpack or hash the uploaded ZIP.
No new blocking finding in this evidence-only follow-up. Please keep the PR draft until the work-package owner records adopt/adapt/reject and the runtime, release/profile, and calibration decisions are settled. Acceptance still needs an actual recorded episode with its native artifacts and matching C9 sidecar. The synthetic handoff verifies the reporting path; its NOT_VERIFIED result does not establish real-episode or production acceptance.
ChatGPT-assisted source and hosted-evidence review; no local ROS, hardware, or real-dataset execution.
Task C10 — Dataset Quality Gate. Owner: SatyaA27. Reviewer: Mohamed Sayed (@panthera-momagdii). Target: 16 October 2026.
DRAFT — SYNTHETIC QA PASS; REAL_EPISODE_NOT_VERIFIED; PRODUCTION ACCEPTANCE BLOCKED.
This adds a deterministic, read-only C9 conformance reference checker. Structural success without required timing, native artifact, calibration/configuration/profile, release or recording evidence cannot open the training gate. PASS, REJECT and NOT_VERIFIED remain separate; exit codes stay 0, 1 and 2. No recorder, training runtime or second native dataset format is introduced.
Evidence-handoff review follow-up
Addresses Mohamed's scoped review at
8440f4b818f4bd0e6bbbbcc1538eee0555e0012a. New signed commit34fd6de3fa9735ee550c2da4da28b48e31bc1f9dchanges only the workflow, evidence generator/tests and handoff documentation. The accepted C9 schema, checker/semantic logic, synthetic test policy, original 94 C10 tests, dependency manifests and existing ROS gates are unchanged by this follow-up.Accessible bundle: download C10 CLI report and native-suite output from successful exact-head C10 run 38938591049. Artifact:
c10-evidence-34fd6de3fa9735ee550c2da4da28b48e31bc1f9d-1, 16 files, retained 30 days (expires 15 November 2026). GitHub authentication may be needed. Reviewers no longer need access to ignored local paths.The downloaded artifact was independently verified after CI:
synthetic/evidence.json: actual, expected and after-run checker SHA all34fd6de3fa9735ee550c2da4da28b48e31bc1f9d; source tree clean before/after; immutable checkout verified.openamrobot/synthetic-c9-contract-fixture, revisionsynthetic-fixture-1, episodesynthetic-episode-000000, index 0, origin synthetic.synthetic/c10-report.json: unchanged raw CLI NOT_VERIFIED / exit 2,training_ready=false,production_acceptance=NOT_VERIFIED; 134 PASS + 22 NOT_VERIFIED findings, native fixture coverage 2/2 and selected-time skew 0 ns on two synthetic frames.d5280ecbfaa4ccd5da4d7f72aac24b6adb3a78ef295a524544df19114389abbe, identical to the clean-head local report. Archive SHA-256 matches GitHub's published digest:03659517404adfad6d2cb604136c92a49af2b55981716b8259b822eb67d489f8.core-suite.log,native-suite.log,evidence-suite.log: 63 / 31 / 23 tests, each OK, zero unittest skips. The native log confirms the actual synthetic MP4 creation/decode/corruption branch; this is not a robot recording.generation.log, original CLI stdout/stderr, revert-negative output, native Parquet, matching C9 sidecar and synthetic policy are included.The helper's exit 0 means the expected blocked reporting path was observed. It does not rewrite the report or convert exit 2 into training readiness. Unexpected PASS/REJECT, inconsistent or missing reports, source/data drift and wrong/dirty expected checkouts fail the helper and retain available raw evidence.
Captured test/helper pipelines use
pipefail; later evidence steps run after earlier failures unless cancelled, and upload usesif: always()with hidden files included. Local regression tests execute all five captured command pipelines with injected exit 23, verify logs remain and exit 23 is preserved, and enforce unconditional upload configuration. Nocontinue-on-erroror blanket|| trueis used. The successful hosted run proves accessible upload on this head; no deliberately failing hosted run is claimed.Not verified
The full October 16 deliverable, including one real episode and production acceptance, is not satisfied by this handoff.
Integration Gate
Searched the authorized workspace, accessible organization repository list, organization C10/dataset issues and PR references, interfaces open PRs, accepted C9 issue #18/comments (including 13 October), merged PR #16, organization AGENTS/decisions and the existing baseline. No overlapping open interfaces PR or approved Embodied AI/C10 runtime home was found. Search absence is not proof that private decisions do not exist.
Reused unchanged: accepted C9 Draft 2020-12 schema; existing
lint_schemas.load_dataduplicate-key/nonfinite validation path; existing unittest runner/zero-test/no-skip guard; native LeRobot format/artifact bindings. The positive fixture is derived from the real repository C9 YAML and changes only its temporary copied synthetic data/policy, including the event interval needed to contain linked frame 0.Inspected read-only:
4u-79/openamrobot-c-baselineata29ce7c89b7c42c9147ad81e6a751de9dad045c9, especially both validators, eight-fixture harness, info, requirements and README. Useful corruption concepts informed independent checks. No baseline code was copied or changed: that commit has no tracked source licence. Its 19-channel historical fixed-mast constants, camera aliases, epoch comparisons, 30 ms/95% FPS defaults and nonblocking stale-image flags were rejected as accepted C9 policy.Placement: interfaces remains canonical contract owner; C10 is a consumer under existing
tools/andtests/as a draft conformance reference pending architecture review. No runtime ownership is invented; no ROS recorder, training, motion, safety or governance implementation is added. C9 schema/docs/fixtures, native format and frozen contracts are unchanged.Follow-up reuse: the existing C9-derived fixture/native writer supplies the evidence inputs unchanged. The generator launches the original checker as a subprocess and inventories exact input/output bytes. No accepted contract, policy values, runtime ownership, dependency pin or baseline source was changed in response to the review.
Compatibility and unresolved decisions
The compatibility matrix compares accepted C9 and the baseline across cameras, ordered/versioned vectors and units, clock domains/mapping, sample time, skew/action latency, release provenance, calibration/configuration hashes and native bindings.
head/baseare required;camera_scene/camera_base_navare never silently aliased.lerobot==0.4.1are not approved project release pins. External snapshot identity evidence can bind standard native metadata without injecting nonnative fields intoinfo.json.File-by-file changes
tools/check_c10_dataset.pytools/c10_semantics.pytools/generate_c10_evidence.pytests/test_c10_evidence.pydocs/c10-evidence-handoff.mdtools/prove_c10_negative.pytests/c10_fixture.pytests/test_c10_dataset.pytests/test_c10_native.pytools/requirements-c10.txttools/requirements-c10-native.txttools/verify.sh.github/workflows/c10-dataset-quality.ymldocs/c10-readme.mddocs/c10-compatibility-matrix.mddocs/c10-provenance.mddocs/c10-check-fixture-matrix.mddocs/c10-verification.mdTests
Branch base:
87ab8a936c97ab55ee74b8988ca3d7659160ea6a.Follow-up parent:
8440f4b818f4bd0e6bbbbcc1538eee0555e0012a.Current head:
34fd6de3fa9735ee550c2da4da28b48e31bc1f9d.Branch:
SatyaA27:c10-dataset-quality-gate→openAMRobot/openamrobot-interfaces:main.Verified hosted results for current head
PASS: all implemented verification stages.quality/testjob is SKIPPED and is not counted as test evidence.C10 workflow checks out the explicit PR-head SHA and requires it in the generator, rather than describing a merge checkout as the tested checker revision. Hosted C10 environment: Python 3.12.15 on Ubuntu 24.04; jsonschema 4.26.0, PyYAML 6.0.3, pyarrow 26.0.0. The artifact lists complete installed package versions. Local environment: macOS 26.5.1 arm64, Python 3.12.13 with the same three package versions.
Exact local commands executed from repository root:
The full existing schema suite was also executed locally with
.../python tools/run_verification_tests.py tests test_schema_validation.py: 15 tests, FAILED with 3 missing-catkin_pkg errors. It is not a local pass. All 15 passed in hosted Jazzy. Localbash tools/verify.shwas not run without Jazzy/colcon; hostedquality/buildran that full command successfully.Additional follow-up revert proof: in a temporary helper source copy only, replace
if not expected_gate_result(report, manifest["gate"]["process_exit_code"]):withif False:and run unchangedC10EvidenceTests.test_rejects_unexpected_gate_pass. It failed exactly once atassertNotEqual(helper_exit, 0)because the reverted guard accepted an unexpected PASS. No import errors/skips; original tracked source and test expectations were preserved. The 23-test suite includes that unchanged negative test on the real helper, source/data/HEAD drift, exact exit-code types, report reproduction, fresh-directory protection and retained failure evidence.Exact checker command captured by the hosted manifest (cwd
/home/runner/work/openamrobot-interfaces/openamrobot-interfaces):For another machine, use a clean checkout at
34fd6de3fa9735ee550c2da4da28b48e31bc1f9d, install the recorded dependency versions, then run:Output must be fresh. The generated manifest captures the new machine's actual interpreter/paths; raw report and fixture reproduction remain separate from invocation-specific timestamp/path fields. Handoff documentation describes every artifact and failure behavior. Earlier local ignored evidence remains supplemental, not the reviewer handoff.
Dependencies and licences
No training stack/LeRobot release pin or unrelated dependency was changed.
jsonschema>=4.18,<5(PyPI, MIT) andPyYAML>=6,<7(PyPI, MIT) expose existing standalone schema prerequisites. Optionalpyarrow>=17,<27(PyPI/Apache Arrow, Apache-2.0) supports native inspection. External ffmpeg/ffprobe are distribution-installed only for media tests (LGPL/GPL depending on build); no binary vendored. Software is MIT and documentation CC BY 4.0 under the existing licence map. Baseline source was not copied because licence permission is unresolved. Provenance and modifications are documented.This follow-up adds no dependency or manifest change; it reuses the installed native-test environment and standard library. New helper/tests carry MIT SPDX headers; handoff documentation is CC BY 4.0 under the existing licence map.
Intellectual property and provenance
AI disclosure
OpenAI Codex produced the draft checker, synthetic fixtures/tests, CI wiring and documentation with multi-agent implementation and adversarial review. As requested, the semantic agent used
gpt-6-astrawith high reasoning; test work usedgpt-6.1-solwith high reasoning; audit/documentation usedgpt-6.1-solwith medium reasoning. Human approval, hardware evidence, contributor-agreement acceptance and release acceptance were not generated or inferred.Follow-up implementation also used OpenAI Codex agents for the evidence generator, 23 regression tests, workflow/documentation updates and adversarial review. No human adoption decision, release/timing/action/calibration approval, real recording or agreement attestation was inferred.
Owner and unresolved decisions
One checklist remains open; no pending item is filled with baseline defaults:
@panthera-momagdii: evidence handoff is now accessible and reproducible. Please review these outputs and pending decisions. The prior formal reviewer request was denied by GitHub permission; no reviewer assignment or approval is fabricated. Keep draft; no merge or ready-for-review transition is requested without the user's review.