Skip to content

docs: correct claims that the code contradicts - #1011

Open
ivany-nv wants to merge 6 commits into
mainfrom
ivany-nv/doc-factual-corrections
Open

docs: correct claims that the code contradicts#1011
ivany-nv wants to merge 6 commits into
mainfrom
ivany-nv/doc-factual-corrections

Conversation

@ivany-nv

@ivany-nv ivany-nv commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Eleven corrections across published docs and agent notes, each verified against the implementation rather than inferred. Docs only — no code, no build config.

Things that raise if you paste them

  • teleop_session.rstoxr.OpenXRSession.create(...). There is no create; the binding exposes only the ctor (oxr_bindings.cpp:103-105). A static Create existed and was deliberately replaced in 6c42522b6 ("Use OpenXRSession ctor instead of static Create", Use OpenXRSession ctor instead of static Create #99, 2026-02-09). A repo-wide sweep found this to be the only doc line that change left behind.
  • teleop_session.rst — the Simplified Gripper Example is listed under examples/retargeting/python/, where it has never lived. It is examples/teleop/python/gripper_retargeting_example_simple.py.
  • retargeting/index.rst — readers are told to implement compute(). That is concrete on BaseRetargeter — it fills optional inputs and syncs parameters before delegating. The abstract hook is _compute_fn() (base_retargeter.py:126-128), and nothing in the tree overrides compute(). A subclass following the doc cannot be instantiated.
  • build.rstOXR / DEVICEIO / schema given as example CMake targets. None of those names exist; the exported aliases are oxr::oxr_core, deviceio::deviceio_session, isaacteleop_schema.

Device reference tables

  • trackers.rst — four #include <deviceio/...> paths. There is no deviceio C++ include root; the only deviceio directory in the tree is the Python package. deviceio_trackers publishes .../cpp/inc, so these resolve as <deviceio_trackers/...> — which two rows in the same table already used. Those four would not compile.
  • trackers.rstGeneric3AxisPedalTracker listed one record channel. Its manifest entry sets channel = "pedals" with no mcap_channels override, so it resolves through [defaults.pull] to pedals and pedals_tracked, exactly like frame_metadata_oak two rows above, which lists both. Code written against the documented single channel silently misses half the data.
  • manus.rst — the plugin header is shown at inc/core/...; it is at core/inc/manus/.... The two path components were transposed. Added the sibling header that lives there too, so the listing matches disk.

Agent notes

  • deviceio_trackers/AGENTS.md — a relative link resolving to the repo root. The file moved under src/python/ in 4bff0fd69, and the prose in that same bullet was updated for the move while the link was not.
  • replay_trackers/AGENTS.md — the hand-written replay impl list omitted haptic_command_reader, which qualifies on every test the sentence implies: git-tracked, no AUTO-GENERATED banner, named in replay_trackers/cpp/CMakeLists.txt, absent from trackers.toml.
  • src/core/AGENTS.mdIDeviceIOSource leaves were described as discoverable only via a declared OutputCombiner output. _discover_sources unions three roots — the main pipeline, teleop_control_pipeline, and every registered sink subgraph — so a source feeding only a sink is discovered with no combiner involved (teleop_session.py:251-267, locked in by test_sink_subgraph_source_is_discovered).
  • docs/AGENTS.md and ecosystem.rst — both quoted #partners-15. partners.yaml holds nine active records, so it is #partners-9 today and something else after the next card. Hardcoding a number inside an explanation of why that number keeps changing is the defect, so both now describe the slug without quoting a count.

Not included

src/viz/AGENTS.md had the worst drift of the eleven agent-note files — 13 of 68 falsifiable claims false — and is deliberately untouched here. That module is mid-refactor (ten commits in the two months to 2026-08-24, one the same day), so a correction written now is likely stale before it is reviewed. The findings are recorded and will be re-run against the code once that work settles.

Summary by CodeRabbit

  • Documentation
    • Corrected SDK header paths and CMake library target names.
    • Updated tracker documentation, including the pedals_tracked channel.
    • Refreshed teleoperation examples and session-construction guidance.
    • Clarified custom retargeter implementation requirements.
    • Improved section-heading, ecosystem-link, and source-discovery guidance.
    • Fixed documentation links and tracker-list formatting.

Each of these was checked against the code and has a single defensible fix, so
they are applied here rather than raised for a decision.

deviceio_trackers/AGENTS.md:30 linked `../../../python/isaacteleop/...`, which
resolves to the repo root — there is no `python/` there. The file moved under
`src/python/` in 4bff0fd, and the prose in that same bullet was updated for
the move while the link was not. Now `../../python/...`.

replay_trackers/AGENTS.md:14 listed the hand-written replay impls and omitted
`haptic_command_reader`. It qualifies on every test the sentence implies: it is
git-tracked, carries no AUTO-GENERATED banner, is named in
replay_trackers/cpp/CMakeLists.txt:14, and is absent from trackers.toml. The
manifest's `haptic_command` entry generates a differently named file
(replay_haptic_command_tracker_impl), so the two are not the same thing.

docs/AGENTS.md:95 claimed every section on the ecosystem page carries a
`.. _label:` target. Four of five do; `Become a Partner` (ecosystem.rst:84) has
none, and it is also the only heading with no count pill — so the stated
rationale never applied to it. Narrowed the claim to counted sections and named
the exception, rather than adding a target the page does not need.

Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
src/core/AGENTS.md:21 said IDeviceIOSource leaves are discovered only via a
declared OutputCombiner output. _discover_sources unions three roots — the main
pipeline, teleop_control_pipeline, and every registered sink subgraph — so a
source feeding only a sink is discovered with no combiner involved
(teleop_session.py:251-267, locked in by test_sink_subgraph_source_is_discovered).
The rest of the bullet is reworded to match, since "the user's combiner must
include it" no longer followed.

The src/viz/AGENTS.md corrections first drafted here are deliberately deferred.
That module is under active refactor — ten commits in the last two months, the
most recent the same day this was written — so a correction landed now is
likely stale before it is reviewed. The eleven findings are recorded in the
audit report and should be re-run against the code once that work settles.

Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
Four corrections, each verified against the implementation. Every one raises or
fails if a reader pastes it.

teleop_session.rst: `oxr.OpenXRSession.create("MyApp", exts)`. There is no
`create` — the binding exposes only
`py::init<const std::string&, const std::vector<std::string>&>`
(oxr_bindings.cpp:103-105). A static `Create` did exist and was deliberately
replaced in 6c42522 ("Use OpenXRSession ctor instead of static Create", #99,
2026-02-09). A repo-wide sweep found this to be the only doc line that change
left behind; four real call sites already use the ctor. AttributeError as
written.

teleop_session.rst: the Simplified Gripper Example is listed at
`examples/retargeting/python/gripper_retargeting_simple.py`, which does not
exist. The file is `examples/teleop/python/gripper_retargeting_example_simple.py`
— both the directory and the name differ, and `examples/retargeting/python/`
holds only retargeter demos, so this reads as wrong from the start rather than
a rename that was missed.

retargeting/index.rst: readers are told to implement `compute()`. That is
concrete on `BaseRetargeter` — it fills optional inputs and syncs parameters
before delegating. The abstract hook is `_compute_fn()` (base_retargeter.py:126-128),
and no retargeter in the tree overrides `compute()`. A subclass following the
doc cannot be instantiated.

build.rst: `OXR`, `DEVICEIO` and `schema` are given as example targets. None of
those names exist. The libraries export namespaced aliases — `oxr::oxr_core`,
`deviceio::deviceio_session`, `isaacteleop_schema` — which is what the examples
under examples/ actually link.

Two corrections that were in an earlier version of this commit are gone: 8278e3f
rewrote the Quick Start gripper block to the two-retargeter + OutputCombiner
shape, which fixed the missing `config` argument and made `result["gripper_left"]`
correct — that key is now genuinely produced by the OutputCombiner, so the change
this commit used to make would have reintroduced a KeyError.

Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
trackers.rst gave `#include <deviceio/...>` for head, hand, controller and
generic_3axis_pedal. There is no `deviceio` C++ include root — the only
`deviceio` directory in the tree is the Python package
src/python/isaacteleop/deviceio. deviceio_trackers publishes `.../cpp/inc`
(deviceio_trackers/cpp/CMakeLists.txt:26), so the headers resolve as
`<deviceio_trackers/...>`, which is what the same table already used for
full_body and frame_metadata_oak. Those four would not compile.

trackers.rst:255 listed one record channel for Generic3AxisPedalTracker. Its
manifest entry sets `channel = "pedals"` and no `mcap_channels` override, so it
resolves through `[defaults.pull]` — `["%channel%", "%channel%_tracked"]` —
exactly like frame_metadata_oak, whose row two entries above correctly lists
both. Code written against the documented single channel misses
`pedals_tracked`.

manus.rst showed the plugin header at `inc/core/manus_hand_tracking_plugin.hpp`.
It is at `core/inc/manus/manus_hand_tracking_plugin.hpp`; the tree had the two
path components transposed. Added the sibling header that lives there too, so
the listing matches what is on disk.

Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
Both places said `#partners` becomes `#partners-15`. partners.yaml holds nine
`active` records, so it is `#partners-9` today and something else after the next
card lands.

Refreshing the digits would buy one cycle. Hardcoding a number inside an
explanation of why that number keeps changing is the defect, so both now
describe the slug without quoting a count.

docs/AGENTS.md is the rule; ecosystem.rst carries the same sentence as an RST
comment above the targets it justifies. They drifted together and are fixed
together.

Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d6568a74-6e5e-4238-af6b-14a373bc6f79

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f05a6a5b-a07a-4401-b6e0-e8012f864f6a

📥 Commits

Reviewing files that changed from the base of the PR and between 8278e3f and 4f88a7a.

📒 Files selected for processing (10)
  • docs/AGENTS.md
  • docs/source/device/manus.rst
  • docs/source/device/trackers.rst
  • docs/source/getting_started/teleop_session.rst
  • docs/source/overview/ecosystem.rst
  • docs/source/references/build.rst
  • docs/source/references/retargeting/index.rst
  • src/core/AGENTS.md
  • src/core/deviceio_trackers/AGENTS.md
  • src/core/replay_trackers/AGENTS.md

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The pull request updates documentation and repository guidance. It corrects SDK and tracker header paths, CMake target names, example references, OpenXR session construction, retargeter method instructions, and pedal channels. It also clarifies stable section anchors and teleoperation source-discovery rules. Additional agent documentation links and tracker list formatting were corrected.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4f88a

This PR corrects documentation and agent notes without changing code, build configuration, permissions, or runtime behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies documentation corrections to claims that conflict with the implementation. It is concise and relevant to the main changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (10 skipped: 10 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ivany-nv/doc-factual-corrections

Comment @coderabbitai help to get the list of available commands.

@ivany-nv
ivany-nv requested a review from yanziz-nvidia August 25, 2026 04:14
deviceio_trackers/AGENTS.md said every non-required key "has a `%placeholder%`
default in defaults.toml". Most do — schema, class, channel, traits and the
mcap/replay channel lists all derive from `name` or `table` by substitution.
Several do not: direction, shape, python_accessor, max_flatbuffer_size, record
and facade_tensor_constant are plain literals, as are the [defaults.push]
overrides.

Nothing breaks either way; the cost is a reader who opens defaults.toml, finds
`max_flatbuffer_size = 512` where the prose promised a placeholder, and starts
wondering what else in the file is wrong.

Signed-off-by: Ivan Yang <yifanyang@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant