docs: correct claims that the code contradicts - #1011
Conversation
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>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe 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 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)
Full details: Docstring CoverageExplanation 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)
Comment |
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>
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.rst—oxr.OpenXRSession.create(...). There is nocreate; the binding exposes only the ctor (oxr_bindings.cpp:103-105). A staticCreateexisted and was deliberately replaced in6c42522b6("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 underexamples/retargeting/python/, where it has never lived. It isexamples/teleop/python/gripper_retargeting_example_simple.py.retargeting/index.rst— readers are told to implementcompute(). That is concrete onBaseRetargeter— 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 overridescompute(). A subclass following the doc cannot be instantiated.build.rst—OXR/DEVICEIO/schemagiven as example CMake targets. None of those names exist; the exported aliases areoxr::oxr_core,deviceio::deviceio_session,isaacteleop_schema.Device reference tables
trackers.rst— four#include <deviceio/...>paths. There is nodeviceioC++ include root; the onlydeviceiodirectory in the tree is the Python package.deviceio_trackerspublishes.../cpp/inc, so these resolve as<deviceio_trackers/...>— which two rows in the same table already used. Those four would not compile.trackers.rst—Generic3AxisPedalTrackerlisted one record channel. Its manifest entry setschannel = "pedals"with nomcap_channelsoverride, so it resolves through[defaults.pull]topedalsandpedals_tracked, exactly likeframe_metadata_oaktwo rows above, which lists both. Code written against the documented single channel silently misses half the data.manus.rst— the plugin header is shown atinc/core/...; it is atcore/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 undersrc/python/in4bff0fd69, 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 omittedhaptic_command_reader, which qualifies on every test the sentence implies: git-tracked, no AUTO-GENERATED banner, named inreplay_trackers/cpp/CMakeLists.txt, absent fromtrackers.toml.src/core/AGENTS.md—IDeviceIOSourceleaves were described as discoverable only via a declaredOutputCombineroutput._discover_sourcesunions 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 bytest_sink_subgraph_source_is_discovered).docs/AGENTS.mdandecosystem.rst— both quoted#partners-15.partners.yamlholds nineactiverecords, so it is#partners-9today 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.mdhad 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
pedals_trackedchannel.