Core rewrite phase 10: documentation#127
Merged
Merged
Conversation
Docs-only slice of the Phase 10 commit (4aa8098): relocate docs/developer_guide*.rst into docs/dev_guide/ with the dev_guide_<xxx> prefix, plus the matching :doc: cross-reference updates in user_guide files. Non-doc Phase 10 changes (config files, src/, tests/) live on the core_rewrite_phase10 branch instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 10 moved docs/developer_guide*.rst into docs/dev_guide/ but left the internal cross-references pointing at the old prefix. This commit finishes the migration: - Rewrite the toctree entries in dev_guide.rst and dev_guide_navigation_models.rst to use the new dev_guide_<xxx> basenames. - Update :doc: roles in moved files (dev_guide_introduction, dev_guide_orchestrator, dev_guide_extending, dev_guide_rotation, dev_guide_configuration, dev_guide_techniques, dev_guide_class_hierarchy) to drop the developer_ prefix. - Switch :doc: roles that point outside dev_guide/ to absolute paths (/introduction_configuration, /api_reference/api_nav_model) so they still resolve from the new directory. - Update index.rst's top-level toctree entry to dev_guide/dev_guide. - Update :doc: roles in user_guide_navigation.rst to dev_guide/dev_guide_*. sphinx-build -W -b html exits clean; sphinx-build -b linkcheck reports no broken internal links (a single pre-existing external 404 for an unrelated rms-cloud_tasks GitHub URL is independent of this change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sequence the dev_guide pages and src/nav modules a new contributor should read to come up to speed, in ten stages from orientation through extending. Stage 4 walks one image end-to-end through NavModelBody + BodyLimbNav (the smallest autonomous technique and canonical DT-fitting reference); later stages pair each remaining model with its techniques. Every class, function, method, and module reference is a Sphinx cross-reference into the API autodoc; non-autodoc'd paths are flagged inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each comma-separated cluster of classes, methods, and modules in the familiarization plan is now a nested bullet list, so a reader can work through them one at a time instead of parsing prose. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stage 6 subsections and Stage 7 doc list now follow the stars → bodies → rings → titan ordering used elsewhere, with manual after titan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
WalkthroughReorganizes and expands developer documentation into a hierarchical dev_guide/ tree, adds many new developer-guide pages (models, techniques, orchestrator, config/static-data, backplanes, PDS4, image-library, etc.), tightens Sphinx cross-reference/build rules and conf.py options, updates API automodule entries, and makes one small validation change in RingAnnulusNav input checking. ChangesDocumentation + single validation change
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
|
mypy could not see numbers.Real as int-coercible, so int(raw) and the returned value tripped call-overload and no-any-return errors. YAML config only ever produces int / float / bool / str / None, so a concrete (int, float) check is sufficient and types cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rf_core_rewrite #127 +/- ##
==================================================
Coverage ? 54.66%
==================================================
Files ? 135
Lines ? 16893
Branches ? 2283
==================================================
Hits ? 9234
Misses ? 7042
Partials ? 617 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 42
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/dev_guide/dev_guide_reprojection.rst (1)
57-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPublic method
to_bounded()must use a Sphinx cross-reference role, not a bare inline literal.Per the coding guidelines, every mention of a class, method, function, module, attribute, or data constant in narrative prose must use an appropriate Sphinx cross-reference role (
:meth:,:func:, etc.). Bare inline literals are only for YAML/JSON keys, file paths, CLI tokens, and shell snippets.🔧 Proposed fix
-To retrieve data that spans the wraparound boundary, ``to_bounded()`` accepts +To retrieve data that spans the wraparound boundary, :meth:`~nav.reproj.bodies.BodyMosaic.to_bounded` acceptsAs per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use appropriate Sphinx cross-reference roles."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/dev_guide/dev_guide_reprojection.rst` at line 57, Replace the bare inline literal mention of the method with a Sphinx cross-reference role: change the inline ``to_bounded()`` reference to a method role like :meth:`to_bounded` in the docs text so the public method is properly cross-referenced; update any similar occurrences in the same sentence/paragraph to use :meth:`to_bounded` rather than backticks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/conf.py`:
- Around line 53-58: The current html_theme_options sets collapse_navigation =
False together with navigation_depth = -1 which can bloat generated HTML; update
the html_theme_options to use a finite navigation_depth (e.g., 4 or 5) or
re-enable collapse_navigation to avoid expanding every section on every
page—modify the navigation_depth value in the html_theme_options dict (and
optionally set collapse_navigation = True) so the sidebar limits expansion and
keeps HTML sizes reasonable.
In `@docs/dev_guide/dev_guide_annotations.rst`:
- Line 10: Replace the inline API literals in the prose with Sphinx
cross-reference roles: change occurrences of the method mention
"to_annotations(context)" to a :meth: role (e.g. :meth:`to_annotations`), change
any data constant mentions like "TEXTINFO_*" to :data: roles (e.g.
:data:`TEXTINFO_FOO`), and change the internal helper "_collect_annotations" to
the appropriate role (use :func:`_collect_annotations` if it is a function or
:meth:`_collect_annotations` if it is an instance method); update all other
plain symbol mentions in the file to the correct :class:, :func:, :attr:, or
:data: roles per the docs standard so every API symbol in narrative prose uses a
Sphinx cross-reference.
In `@docs/dev_guide/dev_guide_backplanes.rst`:
- Line 97: Replace the bare inline code mentions of the API symbols with Sphinx
cross-reference roles: change ``ObsSnapshot`` to :class:`ObsSnapshot` (or
:class:`module.ObsSnapshot` if it lives in a submodule) and change
``cspyce.bodn2c`` to :func:`cspyce.bodn2c`; update both the occurrence near the
``isinstance(obs, ObsSnapshot)`` sentence and the other occurrence noted (176)
so the narrative uses :class: and :func: roles rather than raw inline literals
and include the full import path if needed for correct linking.
In `@docs/dev_guide/dev_guide_config_and_static_data.rst`:
- Around line 296-301: The :meth:`~nav.config.config.Config._load_yaml`
cross-reference will not resolve because Sphinx omits private members; update
the docs in dev_guide_config_and_static_data.rst to avoid the failing cross-ref
by replacing the Sphinx method role referencing Config._load_yaml with a plain
inline literal (e.g. use ``Config._load_yaml`` or ``_load_yaml``) wherever that
:meth: reference appears so the build no longer errors under sphinx-build -W
while preserving the explanatory text about stripping ``_``-prefixed mapping
keys.
In `@docs/dev_guide/dev_guide_extending.rst`:
- Around line 189-190: Replace the inline literal mention of
NavTechnique._registry in the narrative text with a Sphinx attribute
cross-reference using the :attr: role; locate the sentence that currently
contains `NavTechnique._registry` and change it to use
:attr:`NavTechnique._registry` so the attribute is properly linked per the
documentation guidelines.
In `@docs/dev_guide/dev_guide_familiarization.rst`:
- Around line 373-374: Replace the inline literal references ``BodyMosaic`` and
``RingMosaic`` with proper Sphinx class cross-references: use
:class:`nav.reproj.bodies.BodyMosaic` for the BodyMosaic reference and
:class:`nav.reproj.rings.RingMosaic` for the RingMosaic reference so they are
autodoc-linked and do not trigger nitpicky-mode warnings; update the two list
items where ``BodyMosaic`` and ``RingMosaic`` appear in the
dev_guide_familiarization.rst content accordingly.
In `@docs/dev_guide/dev_guide_introduction.rst`:
- Around line 19-22: The module names in the paragraph are currently written as
inline literals (e.g., ``NumPy``, ``SciPy``, ``oops``, ``starcat``,
``psfmodel``, ``filecache``, ``PyQt6``); change each to the appropriate Sphinx
module role (e.g., :mod:`numpy`, :mod:`scipy`, :mod:`oops`, :mod:`starcat`,
:mod:`psfmodel`, :mod:`filecache`, :mod:`PyQt6`) so narrative prose uses :mod:
cross-references consistently (replace the backtick-wrapped names with
:mod:`...` roles and keep the surrounding text unchanged).
In `@docs/dev_guide/dev_guide_logging.rst`:
- Around line 95-96: The Sphinx role is incorrect: replace the
:class:`~nav.nav_orchestrator.nav_result.NavResult.status_reason` usage with the
attribute role :attr:`~nav.nav_orchestrator.nav_result.NavResult.status_reason`
so Sphinx treats status_reason as an attribute of NavResult; update any other
occurrences in the same doc to use :attr: for attributes and then validate by
building with nitpicky mode (sphinx-build -n -b html) to ensure no warnings
remain.
In `@docs/dev_guide/dev_guide_navigation_models_body_simulated.rst`:
- Around line 27-29: Replace the inline literal ``BODY_DISC`` in the narrative
with a Sphinx data-role cross-reference (use :data:`...`) to match the usage on
Line 12; specifically change the backticked literal to the same
:data:`BODY_DISC` reference (using the same fully-qualified target/module used
earlier in the file) so the constant is referenced with the data role rather
than a plain inline code literal.
In `@docs/dev_guide/dev_guide_navigation_models_body.rst`:
- Around line 454-455: Replace the bare inline-literal API mentions with proper
Sphinx cross-reference roles: change ``NavModel._registry`` to
:attr:`NavModel._registry`, ``__init_subclass__`` to :meth:`__init_subclass__`
(or :meth:`NavModel.__init_subclass__` for clarity), the reference to
:func:`~nav.nav_model.nav_model.build_models_for_obs` is fine but ensure
consistency, and convert ``_compute_limb_mask_from_body_mask``,
``_create_annotations``, and ``LIMB_ARC`` to the appropriate roles (:func: or
:meth: for private functions/methods like
:meth:`_compute_limb_mask_from_body_mask` / :meth:`_create_annotations`, and
:data:`LIMB_ARC` or :attr:`LIMB_ARC` for the constant/enum member) so all API
symbols use Sphinx cross-reference roles consistently throughout the document.
In `@docs/dev_guide/dev_guide_navigation_models_ring_simulated.rst`:
- Line 28: Replace the incorrect Sphinx role :attr: with the :data: role for the
symbol NavFeatureType.RING_ANNULUS in the documentation line that currently
reads ":attr:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS`"; update it
to use ":data:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS`" so the
symbol is consistently referenced as a data constant and matches other
occurrences on the page.
- Around line 23-25: The sentence describing RingFeature is malformed; rephrase
it so the subject and verbs are clear — e.g., state that the catalog-driven path
uses the RingFeature data model to carry per-edge metadata and that only the
rendering pipeline differs (pixel-space here vs. backplane-based for the real
model). Update the sentence mentioning
:class:`~nav.nav_model.rings.ring_feature.RingFeature` to remove the duplicated
verbs ("uses carries") and ensure it reads as a single clear clause conveying
that RingFeature carries per-edge metadata while rendering differs.
In `@docs/dev_guide/dev_guide_navigation_models_ring.rst`:
- Around line 11-12: Replace inline code literals in the prose with Sphinx
cross-reference roles: change ``RingFeatureFilter`` to
:class:`RingFeatureFilter`, ``_create_edge_annotations`` to
:meth:`_create_edge_annotations`, and ``compute_antialiasing`` to
:func:`compute_antialiasing`; keep the existing
:data:`~nav.feature.feature_type.NavFeatureType.RING_EDGE` usage. Apply the same
role conversions for all other occurrences noted in the comment so every class,
method, and function mention uses the appropriate :class:, :meth:, or :func:
role for proper linking in the docs build.
In `@docs/dev_guide/dev_guide_navigation_models_rings.rst`:
- Around line 24-25: The listed inline literal module names (``ring_types``,
``ring_feature``, ``ring_filter``, ``ring_math``, ``ring_render_context``,
``ring_render_result``) should be converted to Sphinx :mod: cross-reference
roles (e.g. :mod:`ring_types`) in the sentence, or if any of those identifiers
are not actual modules, reword the phrase to avoid a module reference and
instead use plain text (e.g. "the ring_types component") — update the text where
those symbols appear so every module mention uses :mod: or is rephrased
accordingly.
In `@docs/dev_guide/dev_guide_navigation_models_star_simulated.rst`:
- Line 8: Replace the bare inline literal ``NavModelStarsSimulated`` with a
Sphinx API role so it becomes a proper cross-reference (e.g. use
:class:`NavModelStarsSimulated` or the fully-qualified form
:class:`package.module.NavModelStarsSimulated` to match your project's
convention); update both occurrences (the one at the current location and the
other occurrence referenced in the comment) so they follow the guideline against
bare CamelCase in prose.
In `@docs/dev_guide/dev_guide_navigation_models_stars.rst`:
- Around line 16-18: The text currently uses a bare CamelCase symbol
`NavModelStarsSimulated`; replace it with the correct Sphinx role or plain
prose: if `NavModelStarsSimulated` is an actual API class, change the mention to
use the class cross-reference role (e.g. :class:`NavModelStarsSimulated`) and
keep the rest of the sentence, otherwise reword the sentence to avoid a bare
symbol (e.g. "a simulated-stars navigation model" or "the simulated-image GUI
variant (not yet implemented)") and ensure the referenced document
:doc:`dev_guide_navigation_models_star_simulated` remains accurate.
In `@docs/dev_guide/dev_guide_navigation_models_titan_simulated.rst`:
- Around line 8-9: Replace bare narrative mentions of NavModelTitanSimulated and
__init_subclass__ with Sphinx cross-reference roles: use
:class:`!NavModelTitanSimulated` for the placeholder class (include the leading
! if the class is intentionally not implemented yet) and use
:meth:`NavModelTitanSimulated.__init_subclass__` (or :meth:`__init_subclass__`
if you prefer the shorter target) for the dunder method; apply the same change
to the other occurrence of these symbols elsewhere in the doc so every
class/method mention uses the appropriate :class:/:meth: roles.
In `@docs/dev_guide/dev_guide_navigation_models_titans.rst`:
- Around line 16-17: The text uses the bare CamelCase symbol
NavModelTitanSimulated in narrative prose; either convert it to a proper Sphinx
class cross-reference (:class:`NavModelTitanSimulated`) if this is an actual API
class, or reword the sentence to neutral prose (e.g., “the simulated Titan
variant” or “a simulated-image GUI variant”) if it’s just a planned placeholder;
update the sentence containing NavModelTitanSimulated accordingly and ensure the
chosen form follows the project’s Sphinx role guideline.
In `@docs/dev_guide/dev_guide_navigation_models.rst`:
- Around line 17-19: Replace the bare inline literals with Sphinx
cross-reference roles: change occurrences of "__init_subclass__" to a method
role like :meth:`__init_subclass__`, change "NavModelStarsSimulated" and
"NavModelTitanSimulated" to class roles like :class:`NavModelStarsSimulated` and
:class:`NavModelTitanSimulated` (use :class:`!NavModelStarsSimulated` /
:class:`!NavModelTitanSimulated` if they are intentionally unresolved
placeholders), and change the narrative mention of NavModel.instances_for_obs to
:meth:`~nav.nav_model.nav_model.NavModel.instances_for_obs`; apply the same
pattern to the other mentions referenced (lines 27 and 41) so every class/method
mention uses the appropriate :class:/ :meth: role.
In `@docs/dev_guide/dev_guide_navigation.rst`:
- Around line 8-10: The prose uses bare CamelCase terms "NavModels" and
"NavTechniques"; replace those with proper Sphinx cross-references to the actual
classes or use neutral wording. Edit the sentence to reference the classes using
roles like :class:`~nav.nav_model.nav_model.NavModel` and
:class:`~nav.nav_technique.nav_technique.NavTechnique` (or convert to neutral
phrases such as “navigation models” and “navigation techniques”) so every API
class mention uses the appropriate cross-reference role instead of bare
CamelCase.
In `@docs/dev_guide/dev_guide_orchestrator_curator.rst`:
- Around line 20-21: Replace inline literal mentions of the constants with
proper Sphinx cross-reference roles: change occurrences of JSON_INF_SENTINEL to
a :data:`JSON_INF_SENTINEL` reference and CURATOR_FIELDS to a
:data:`CURATOR_FIELDS` (or :attr:`CURATOR_FIELDS` if it is an attribute on a
class/module) in the prose around the described blocks (also update similar
occurrences at the ranges noted: lines ~41-47, 74-75, 95-98). Ensure any other
mentions of navigation_result or other module constants use the appropriate role
(:data: or :attr:) so links resolve consistently and follow the documentation
guideline.
In `@docs/dev_guide/dev_guide_orchestrator_ensemble.rst`:
- Line 167: The example lead-ins in the docs use two spaces after the
sentence-ending period; update each affected sentence (e.g., "Two agreeing
techniques. Pass 1 produces" and the similar lines referenced later) to use a
single space after the period so they read "Two agreeing techniques. Pass 1
produces" (and analogous fixes for the other example sentences), ensuring the
repo docs style of exactly one space between sentences.
In `@docs/dev_guide/dev_guide_orchestrator_feature_summary.rst`:
- Around line 22-24: Replace plain literal mentions of BODY_DISC, LIMB_ARC, STAR
(and any other constant names in the same section/lines 102–137) with Sphinx
data cross-references using the :data: role; e.g. change occurrences like
BODY_DISC to :data:`BODY_DISC` or, if needed for clarity/resolution, to
fully-qualified :data:`package.module.BODY_DISC` (same for LIMB_ARC, STAR),
ensuring every constant mention in the prose/examples uses :data: so Sphinx will
link to the API docs.
In `@docs/dev_guide/dev_guide_orchestrator_instrument_config.rst`:
- Around line 99-100: Replace the inline literal ``DataUnits`` with a proper
Sphinx cross-reference to the public API symbol; locate the occurrence of the
inline literal and change it to an appropriate role such as
:py:data:`~your_package.your_module.DataUnits` (or :py:class:`...` /
:py:obj:`...` as appropriate for how DataUnits is exported) so the documentation
links to the documented API alias rather than using bare CamelCase text.
In `@docs/dev_guide/dev_guide_orchestrator_orchestrator.rst`:
- Around line 27-29: The docs use bare internal symbols instead of Sphinx
cross-reference roles; replace occurrences like `_HARD_FAILURE_TO_REASON` and
`STATUS_REASON_INFO_TEMPLATE` with the appropriate :data: role (e.g.,
:data:`_HARD_FAILURE_TO_REASON` and :data:`STATUS_REASON_INFO_TEMPLATE`), and
change any callback method name mentions to the :meth: role pointing to the
fully-qualified method in NavImageClassifier (e.g.,
:meth:`~nav.nav_orchestrator.image_classifier.NavImageClassifier.your_callback_method`);
apply the same conversion to the other affected spots (lines referenced 82–85,
196, 223–224, 260) so all class/method/attribute/data names use the correct
Sphinx roles.
In `@docs/dev_guide/dev_guide_orchestrator_provenance.rst`:
- Around line 27-28: Replace plain inline occurrences of the module-level
constant _STATIC_DATA_PREFIXES in the narrative prose with a Sphinx data
cross-reference using the :data: role (e.g. :data:`_STATIC_DATA_PREFIXES`) so
all mentions (including the ones around lines referenced later at the other
occurrence) are proper Sphinx cross-references; update both the occurrence in
the current paragraph and the similar mentions at the other noted location
(lines 64-65) to use :data:`_STATIC_DATA_PREFIXES`.
In `@docs/dev_guide/dev_guide_reprojection.rst`:
- Around line 244-246: The prose uses bare inline literals for the private
helpers orbit_model_to_dict and orbit_model_from_dict; either reference their
fully-qualified private names with Sphinx function roles (e.g.,
:func:`nav.reproj._serialization.orbit_model_to_dict` and
:func:`nav.reproj._serialization.orbit_model_from_dict`) if you want
cross-references, or rewrite the sentence to describe them generically (e.g.,
"via private serialization helpers in nav.reproj._serialization") to avoid bare
CamelCase/module.symbol text in narrative prose and prevent Sphinx warnings for
non-autodoc'ed private symbols.
- Around line 159-161: Replace the bare inline literals with Sphinx
cross-reference roles: change ``image_number`` to :attr:`image_number`, change
``add()`` to :meth:`add` (or :meth:`ClassName.add` if context requires
qualification), and change ``OverflowError`` to :exc:`OverflowError` so the
attribute, method and exception are properly cross-referenced in the docs.
In `@docs/dev_guide/dev_guide_techniques_body_blob.rst`:
- Around line 9-18: Replace inline code literals like ``BODY_BLOB`` and
``LIMB_ARC`` in the narrative prose with Sphinx cross-reference roles (e.g.,
:data:`~<module>.BODY_BLOB` or the appropriate :class:/:data: role for the
symbol) so the feature-type API symbols are validated and discoverable during
docs builds; update the occurrences in the paragraph that describes
brightness-weighted moments and feasibility (and the other mentions at lines
referenced in the review) to use the correct role for each symbol, ensuring you
point to the canonical import path or symbol name used in the project’s API docs
(use :data: for constants/data and :class:/:func: if the symbol is a
class/function).
In `@docs/dev_guide/dev_guide_techniques_body_disc.rst`:
- Around line 252-253: The sentence "**No rotation fit.** Run
:func:`~nav.support.correlate.navigate_with_pyramid_kpeaks` once on the
unrotated composite..." contains two spaces after the period; change it to a
single space so it reads "**No rotation fit.** Run
:func:`~nav.support.correlate.navigate_with_pyramid_kpeaks`..." by editing the
paragraph that refers to navigate_with_pyramid_kpeaks and normalizing the
spacing after the sentence-ending period.
- Around line 200-214: Replace the bare inline API symbols with Sphinx
cross-reference roles: change ``__init_subclass__`` to a method role (e.g.
:meth:`~nav.nav_technique.nav_technique.NavTechnique.__init_subclass__`), change
the bare ``NavTechnique._registry`` to an attribute role (e.g.
:attr:`~nav.nav_technique.nav_technique.NavTechnique._registry`), and change the
constant ``BODY_DISC`` to a data role (e.g.
:data:`~nav.nav_technique.nav_technique_body_disc.BODY_DISC`); keep the existing
:class: role for BodyDiscCorrelateNav and update any other inline CamelCase API
mentions in this paragraph to the appropriate :class:, :meth:, :attr:, or :data:
roles to satisfy the cross-reference guideline.
- Around line 18-20: Replace the inline code literals for the data constants
BODY_DISC, LIMB_ARC, and BODY_BLOB in the prose with Sphinx :data:
cross-reference roles (e.g. :data:`BODY_DISC`, :data:`LIMB_ARC`,
:data:`BODY_BLOB`) so they become proper linkable API references; update the
sentence that currently uses ``BODY_DISC`` / ``LIMB_ARC`` / ``BODY_BLOB`` to use
the :data: roles for each constant (preserving the existing wording and
punctuation).
In `@docs/dev_guide/dev_guide_techniques_body_limb.rst`:
- Around line 18-21: Replace inline literal uses of the LIMB_ARC data constant
in narrative prose with the Sphinx data-role cross-reference
:data:`~nav.feature.feature_type.NavFeatureType.LIMB_ARC`; locate occurrences
where ``LIMB_ARC`` appears (including the example block and the other listed
occurrences) and update them to use the cross-reference so Sphinx will link to
the NavFeatureType.LIMB_ARC constant.
In `@docs/dev_guide/dev_guide_techniques_body_terminator.rst`:
- Around line 20-23: Replace inline-literal mentions of the data constant
TERMINATOR_ARC in narrative prose with the Sphinx data-role cross-reference
:data:`TERMINATOR_ARC`; update the instance in the provided paragraph (the lines
using ``TERMINATOR_ARC``) and similarly fix the other occurrences in this file
(the noted later mentions) and the other technique docs so all data-constant
mentions use :data: rather than inline literals.
In `@docs/dev_guide/dev_guide_techniques_feasibility.rst`:
- Around line 9-13: Replace the inline literal references to the methods by
using Sphinx method cross-reference roles instead: change occurrences of
``is_feasible`` and ``navigate`` in the prose to :meth:`is_feasible` and
:meth:`navigate` (or to their fully qualified forms like
:meth:`package.module.Class.is_feasible` / :meth:`package.module.Class.navigate`
if necessary for disambiguation), and apply the same replacement for the other
occurrences noted (lines 45–47) so the narrative uses :meth: roles rather than
bare inline literals.
In `@docs/dev_guide/dev_guide_techniques_image_derivatives.rst`:
- Around line 168-170: The Sphinx role for NavFilterKind is incorrect: change
the reference using the :data: role to use the :class: role so the Enum subclass
NavFilterKind is documented as a class (matching NavFilterSpec which already
uses :class:); locate the line containing
":data:`~nav.support.filters.NavFilterKind`" and replace ":data:" with ":class:"
so the reference reads ":class:`~nav.support.filters.NavFilterKind`".
In `@docs/dev_guide/dev_guide_techniques_manual.rst`:
- Around line 86-87: Replace bare module.symbol and CamelCase mentions in the
prose with proper Sphinx API roles: change plain `_abstract` to
:attr:`_abstract`, `NavTechnique._registry` to :attr:`NavTechnique._registry`,
and the module constant ``_MANUAL_OFFSET_SIGMA_PX`` to
:data:`_MANUAL_OFFSET_SIGMA_PX` (or :attr:`` if preferred) so the API-reference
style rule is satisfied; also apply the same substitutions for the other
occurrences noted (around the later mention at lines ~119-121) to maintain
consistent cross-referencing.
In `@docs/dev_guide/dev_guide_techniques_ring_edge.rst`:
- Around line 22-25: The document uses inline literals like ``RING_EDGE`` and
``RING_ANNULUS``; replace every occurrence with the proper Sphinx data
cross-reference roles for the constants, e.g.
:data:`~nav.feature.feature_type.NavFeatureType.RING_EDGE` and
:data:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS`, ensuring
narrative prose uses the cross-reference form (with the full import path)
instead of bare CamelCase inline-literals throughout the file.
In `@docs/dev_guide/dev_guide_techniques_star_field.rst`:
- Around line 215-221: Replace the bare feature-type literals (e.g. STAR,
LIMB_ARC, BODY_BLOB) in narrative prose with Sphinx :data: cross-reference roles
so they follow the documentation rule; update occurrences around the
NavTechnique docs (e.g. the block describing 'StarFieldFromCatalogNav' and
attributes like NavTechnique.accepts_feature_types, NavTechnique.requires_prior,
NavTechnique.confidence_attributes) to use :data:`...` roles pointing to the
actual data constants (use the existing module path for the feature constants),
and make the same replacements in the other affected section (around lines
306–334) so every mention of those feature-type constants in prose uses :data:
roles.
In `@docs/dev_guide/dev_guide_techniques_star_unique_match.rst`:
- Around line 301-302: Replace inline-literal occurrences of ``STAR`` in the
prose with the Sphinx data cross-reference
:data:`~nav.feature.feature_type.NavFeatureType.STAR`; specifically update the
three narrative mentions near the paragraph that references
:class:`~nav.nav_technique.nav_technique_star_unique_match.StarUniqueMatchNav`
so every mention of the STAR feature type uses the :data: role
(NavFeatureType.STAR) instead of backticked literal text.
In `@docs/dev_guide/dev_guide_techniques.rst`:
- Around line 13-15: Replace the bare narrative mention of
NavTechnique._registry with an explicit Sphinx attribute role; locate the
sentence referencing NavTechnique._registry in the docs and change the inline
literal to use the attr role (for example
:attr:`~nav.nav_technique.nav_technique.NavTechnique._registry`) so the registry
attribute is cross-referenced correctly in the rendered docs.
In `@docs/user_guide_navigation.rst`:
- Around line 129-131: Replace the bare inline literals "NavModel" and
"NavTechnique" with proper Sphinx cross-reference class roles (e.g. use :class:
references pointing to the API classes) so the CLI option descriptions use
:class:`...` links instead of plain CamelCase; update the occurrences in the
shown block (the ``--nav-models LIST`` line) and the similar mentions at lines
136–138 to reference the actual fully-qualified class symbols (the NavModel and
NavTechnique classes) so the documentation builds with correct cross-references.
---
Outside diff comments:
In `@docs/dev_guide/dev_guide_reprojection.rst`:
- Line 57: Replace the bare inline literal mention of the method with a Sphinx
cross-reference role: change the inline ``to_bounded()`` reference to a method
role like :meth:`to_bounded` in the docs text so the public method is properly
cross-referenced; update any similar occurrences in the same sentence/paragraph
to use :meth:`to_bounded` rather than backticks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f63d1499-5686-4759-a6d8-fd0cc0dae6ea
📒 Files selected for processing (84)
.cursor/rules/documentation.mdcdocs/api_reference/api_annotation.rstdocs/api_reference/api_nav_orchestrator.rstdocs/api_reference/api_nav_technique.rstdocs/api_reference/api_sim.rstdocs/conf.pydocs/dev_guide/dev_guide.rstdocs/dev_guide/dev_guide_annotations.rstdocs/dev_guide/dev_guide_backplanes.rstdocs/dev_guide/dev_guide_best_practices.rstdocs/dev_guide/dev_guide_class_hierarchy.rstdocs/dev_guide/dev_guide_config_and_static_data.rstdocs/dev_guide/dev_guide_extending.rstdocs/dev_guide/dev_guide_familiarization.rstdocs/dev_guide/dev_guide_image_library.rstdocs/dev_guide/dev_guide_introduction.rstdocs/dev_guide/dev_guide_logging.rstdocs/dev_guide/dev_guide_navigation.rstdocs/dev_guide/dev_guide_navigation_models.rstdocs/dev_guide/dev_guide_navigation_models_bodies.rstdocs/dev_guide/dev_guide_navigation_models_body.rstdocs/dev_guide/dev_guide_navigation_models_body_simulated.rstdocs/dev_guide/dev_guide_navigation_models_ring.rstdocs/dev_guide/dev_guide_navigation_models_ring_simulated.rstdocs/dev_guide/dev_guide_navigation_models_rings.rstdocs/dev_guide/dev_guide_navigation_models_star.rstdocs/dev_guide/dev_guide_navigation_models_star_simulated.rstdocs/dev_guide/dev_guide_navigation_models_stars.rstdocs/dev_guide/dev_guide_navigation_models_titan.rstdocs/dev_guide/dev_guide_navigation_models_titan_simulated.rstdocs/dev_guide/dev_guide_navigation_models_titans.rstdocs/dev_guide/dev_guide_navigation_overview.rstdocs/dev_guide/dev_guide_observations.rstdocs/dev_guide/dev_guide_orchestrator.rstdocs/dev_guide/dev_guide_orchestrator_curator.rstdocs/dev_guide/dev_guide_orchestrator_ensemble.rstdocs/dev_guide/dev_guide_orchestrator_feature_summary.rstdocs/dev_guide/dev_guide_orchestrator_image_classifier.rstdocs/dev_guide/dev_guide_orchestrator_instrument_config.rstdocs/dev_guide/dev_guide_orchestrator_nav_context.rstdocs/dev_guide/dev_guide_orchestrator_nav_result.rstdocs/dev_guide/dev_guide_orchestrator_orchestrator.rstdocs/dev_guide/dev_guide_orchestrator_provenance.rstdocs/dev_guide/dev_guide_pds4.rstdocs/dev_guide/dev_guide_reprojection.rstdocs/dev_guide/dev_guide_rotation.rstdocs/dev_guide/dev_guide_support.rstdocs/dev_guide/dev_guide_techniques.rstdocs/dev_guide/dev_guide_techniques_body_blob.rstdocs/dev_guide/dev_guide_techniques_body_disc.rstdocs/dev_guide/dev_guide_techniques_body_limb.rstdocs/dev_guide/dev_guide_techniques_body_terminator.rstdocs/dev_guide/dev_guide_techniques_confidence.rstdocs/dev_guide/dev_guide_techniques_diagnostics.rstdocs/dev_guide/dev_guide_techniques_dt_fitting.rstdocs/dev_guide/dev_guide_techniques_feasibility.rstdocs/dev_guide/dev_guide_techniques_image_derivatives.rstdocs/dev_guide/dev_guide_techniques_manual.rstdocs/dev_guide/dev_guide_techniques_ring_annulus.rstdocs/dev_guide/dev_guide_techniques_ring_edge.rstdocs/dev_guide/dev_guide_techniques_star_field.rstdocs/dev_guide/dev_guide_techniques_star_refine.rstdocs/dev_guide/dev_guide_techniques_star_unique_match.rstdocs/developer_guide.rstdocs/developer_guide_backplanes.rstdocs/developer_guide_building_docs.rstdocs/developer_guide_class_hierarchy.rstdocs/developer_guide_configuration.rstdocs/developer_guide_extending.rstdocs/developer_guide_navigation_models.rstdocs/developer_guide_navigation_models_bodies.rstdocs/developer_guide_navigation_models_rings.rstdocs/developer_guide_navigation_models_stars.rstdocs/developer_guide_navigation_models_titan.rstdocs/developer_guide_orchestrator.rstdocs/developer_guide_static_data.rstdocs/developer_guide_techniques.rstdocs/developer_guide_uncertainty.rstdocs/index.rstdocs/user_guide.rstdocs/user_guide_backplanes.rstdocs/user_guide_image_library.rstdocs/user_guide_navigation.rstsrc/nav/nav_technique/nav_technique_ring_annulus.py
💤 Files with no reviewable changes (17)
- docs/developer_guide_navigation_models_bodies.rst
- docs/developer_guide.rst
- docs/developer_guide_techniques.rst
- docs/developer_guide_extending.rst
- docs/user_guide.rst
- docs/developer_guide_navigation_models_rings.rst
- docs/developer_guide_orchestrator.rst
- docs/developer_guide_backplanes.rst
- docs/developer_guide_static_data.rst
- docs/developer_guide_navigation_models.rst
- docs/developer_guide_configuration.rst
- docs/developer_guide_uncertainty.rst
- docs/developer_guide_navigation_models_stars.rst
- docs/developer_guide_navigation_models_titan.rst
- docs/developer_guide_building_docs.rst
- docs/developer_guide_class_hierarchy.rst
- docs/user_guide_image_library.rst
Comment on lines
+53
to
+58
| # Show every section level in the sidebar TOC, with all sub-trees expanded. | ||
| html_theme_options = { | ||
| 'navigation_depth': -1, | ||
| 'collapse_navigation': False, | ||
| 'titles_only': False, | ||
| } |
There was a problem hiding this comment.
collapse_navigation: False combined with navigation_depth: -1 may significantly inflate HTML file sizes.
Setting collapse_navigation to False and using a high value for navigation_depth on projects with many files and a deep file structure can cause long compilation times and can result in HTML files that are significantly larger in file size. With this PR adding many new dev-guide pages, using -1 (unlimited depth) is the most extreme case of this tradeoff.
Consider using a finite depth (e.g., 4 or 5) that still exposes the full two-level guide structure without expanding every section heading on every page.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/conf.py` around lines 53 - 58, The current html_theme_options sets
collapse_navigation = False together with navigation_depth = -1 which can bloat
generated HTML; update the html_theme_options to use a finite navigation_depth
(e.g., 4 or 5) or re-enable collapse_navigation to avoid expanding every section
on every page—modify the navigation_depth value in the html_theme_options dict
(and optionally set collapse_navigation = True) so the sidebar limits expansion
and keeps HTML sizes reasonable.
|
|
||
| The :mod:`nav.annotation` subsystem produces the per-image summary-PNG | ||
| overlay. Every :class:`~nav.nav_model.nav_model.NavModel` exposes a | ||
| ``to_annotations(context)`` method that returns a fresh |
There was a problem hiding this comment.
Replace inline API literals with Sphinx API roles
A few API symbols in prose are still plain literals (to_annotations(context), TEXTINFO_*, _collect_annotations). These should be converted to :meth: / :data: references per the docs standard.
As per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use appropriate Sphinx cross-reference roles (:class:, :meth:, :func:, :mod:, :attr:, :data:)".
Also applies to: 39-40, 57-57, 63-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_annotations.rst` at line 10, Replace the inline API
literals in the prose with Sphinx cross-reference roles: change occurrences of
the method mention "to_annotations(context)" to a :meth: role (e.g.
:meth:`to_annotations`), change any data constant mentions like "TEXTINFO_*" to
:data: roles (e.g. :data:`TEXTINFO_FOO`), and change the internal helper
"_collect_annotations" to the appropriate role (use :func:`_collect_annotations`
if it is a function or :meth:`_collect_annotations` if it is an instance
method); update all other plain symbol mentions in the file to the correct
:class:, :func:, :attr:, or :data: roles per the docs standard so every API
symbol in narrative prose uses a Sphinx cross-reference.
| - **Snapshots only.** The pipeline supports | ||
| :class:`~oops.observation.snapshot.Snapshot`-derived observations only; | ||
| push-broom and other observation modes are rejected at the | ||
| ``isinstance(obs, ObsSnapshot)`` check. Adding a non-snapshot path |
There was a problem hiding this comment.
Avoid raw API expressions in prose; cross-reference them.
ObsSnapshot and cspyce.bodn2c are API mentions in narrative text and should be expressed with Sphinx roles (:class:, :func:) instead of raw inline expressions.
As per coding guidelines, “Do not use bare CamelCase or module.symbol text for API symbols in narrative prose, even when wrapped in inline literals”.
Also applies to: 176-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_backplanes.rst` at line 97, Replace the bare inline
code mentions of the API symbols with Sphinx cross-reference roles: change
``ObsSnapshot`` to :class:`ObsSnapshot` (or :class:`module.ObsSnapshot` if it
lives in a submodule) and change ``cspyce.bodn2c`` to :func:`cspyce.bodn2c`;
update both the occurrence near the ``isinstance(obs, ObsSnapshot)`` sentence
and the other occurrence noted (176) so the narrative uses :class: and :func:
roles rather than raw inline literals and include the full import path if needed
for correct linking.
Comment on lines
+189
to
+190
|
|
||
| Operator workflow: |
There was a problem hiding this comment.
Use :attr: for NavTechnique._registry in narrative text.
NavTechnique._registry is an API attribute mention and should be role-linked instead of inline-literal text.
As per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use appropriate Sphinx cross-reference roles".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_extending.rst` around lines 189 - 190, Replace the
inline literal mention of NavTechnique._registry in the narrative text with a
Sphinx attribute cross-reference using the :attr: role; locate the sentence that
currently contains `NavTechnique._registry` and change it to use
:attr:`NavTechnique._registry` so the attribute is properly linked per the
documentation guidelines.
Comment on lines
+22
to
+25
| Feasibility passes when at least one offered ``RING_EDGE`` has a non-empty polyline. A single | ||
| non-empty edge is sufficient — even an all-flat scene produces a useful rank-1 constraint. | ||
| Feasibility fails when every offered ``RING_EDGE`` is empty (a ring system entirely outside the | ||
| extended FOV or below the per-pixel resolution threshold). |
There was a problem hiding this comment.
Use Sphinx cross-references for feature type constants.
Lines 22–25 and 66 use RING_EDGE and RING_ANNULUS in inline-literal form. These are data constants (:data:~nav.feature.feature_type.NavFeatureType.RING_EDGE and :data:~nav.feature.feature_type.NavFeatureType.RING_ANNULUS) and must use Sphinx cross-reference roles in narrative prose per the coding guidelines.
Additional occurrences: lines 178, 191, 200, 226, 247.
📚 Proposed fix for lines 22–25
-Feasibility passes when at least one offered ``RING_EDGE`` has a non-empty polyline. A single
+Feasibility passes when at least one offered :data:`~nav.feature.feature_type.NavFeatureType.RING_EDGE` has a non-empty polyline. A single
non-empty edge is sufficient — even an all-flat scene produces a useful rank-1 constraint.
-Feasibility fails when every offered ``RING_EDGE`` is empty (a ring system entirely outside the
+Feasibility fails when every offered :data:`~nav.feature.feature_type.NavFeatureType.RING_EDGE` is empty (a ring system entirely outside the
extended FOV or below the per-pixel resolution threshold).Line 66:
- model as ``RING_ANNULUS`` templates instead, so the technique never sees them. See
+ model as :data:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS` templates instead, so the technique never sees them. SeeAs per coding guidelines: "Do not use bare CamelCase or module.symbol text for API symbols in narrative prose, even when wrapped in inline literals."
Also applies to: 66-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques_ring_edge.rst` around lines 22 - 25, The
document uses inline literals like ``RING_EDGE`` and ``RING_ANNULUS``; replace
every occurrence with the proper Sphinx data cross-reference roles for the
constants, e.g. :data:`~nav.feature.feature_type.NavFeatureType.RING_EDGE` and
:data:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS`, ensuring
narrative prose uses the cross-reference form (with the full import path)
instead of bare CamelCase inline-literals throughout the file.
Comment on lines
+215
to
+221
| ``'StarFieldFromCatalogNav'``. | ||
| - :attr:`~nav.nav_technique.nav_technique.NavTechnique.accepts_feature_types` — | ||
| ``frozenset({STAR})``. | ||
| - :attr:`~nav.nav_technique.nav_technique.NavTechnique.requires_prior` — ``False``. | ||
| - :attr:`~nav.nav_technique.nav_technique.NavTechnique.confidence_attributes` — | ||
| ``{'at_edge', 'spurious', 'n_inliers', 'median_residual_px', 'n_detected_sources', | ||
| 'n_catalog_predicted'}``. |
There was a problem hiding this comment.
Use :data: roles for feature-type constants in prose
Feature-type constants are referenced as bare/code literals (STAR, LIMB_ARC, BODY_BLOB) in narrative sections. Please link them with :data: roles to align with the documentation rule.
As per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use appropriate Sphinx cross-reference roles (:class:, :meth:, :func:, :mod:, :attr:, :data:)".
Also applies to: 306-334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques_star_field.rst` around lines 215 - 221,
Replace the bare feature-type literals (e.g. STAR, LIMB_ARC, BODY_BLOB) in
narrative prose with Sphinx :data: cross-reference roles so they follow the
documentation rule; update occurrences around the NavTechnique docs (e.g. the
block describing 'StarFieldFromCatalogNav' and attributes like
NavTechnique.accepts_feature_types, NavTechnique.requires_prior,
NavTechnique.confidence_attributes) to use :data:`...` roles pointing to the
actual data constants (use the existing module path for the feature constants),
and make the same replacements in the other affected section (around lines
306–334) so every mention of those feature-type constants in prose uses :data:
roles.
Comment on lines
+301
to
+302
| feature with a brightness margin of ``+inf`` to the (nonexistent) next-brightest. | ||
| :class:`~nav.nav_technique.nav_technique_star_unique_match.StarUniqueMatchNav` runs the |
There was a problem hiding this comment.
Use Sphinx cross-references for the STAR feature type constant.
Lines 301, 313, and 327 use STAR in inline-literal form when referring to the :data:~nav.feature.feature_type.NavFeatureType.STAR data constant. This violates the coding guideline requiring all data constants in narrative prose to use appropriate Sphinx cross-reference roles.
📚 Proposed fix
Line 301:
- Single star (Vega) in an otherwise empty FOV. The stars model emits a single ``STAR``
+ Single star (Vega) in an otherwise empty FOV. The stars model emits a single :data:`~nav.feature.feature_type.NavFeatureType.STAR`
feature with a brightness margin of ``+inf`` to the (nonexistent) next-brightest.Line 313:
- brighter than its next-brightest competing candidate. The stars model emits two
- ``STAR`` features; the technique falls into the 2-star path, runs the per-star local
+ brighter than its next-brightest competing candidate. The stars model emits two
+ :data:`~nav.feature.feature_type.NavFeatureType.STAR` features; the technique falls into the 2-star path, runs the per-star localLine 327:
- the model-side reliability gate, so the orchestrator does not offer any usable star feature to
+ the model-side reliability gate, so the orchestrator does not offer any usable :data:`~nav.feature.feature_type.NavFeatureType.STAR` feature toAs per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use appropriate Sphinx cross-reference roles."
Also applies to: 313-314, 327-328
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques_star_unique_match.rst` around lines 301 -
302, Replace inline-literal occurrences of ``STAR`` in the prose with the Sphinx
data cross-reference :data:`~nav.feature.feature_type.NavFeatureType.STAR`;
specifically update the three narrative mentions near the paragraph that
references
:class:`~nav.nav_technique.nav_technique_star_unique_match.StarUniqueMatchNav`
so every mention of the STAR feature type uses the :data: role
(NavFeatureType.STAR) instead of backticked literal text.
Comment on lines
+13
to
+15
| Concrete subclasses self-register via ``__init_subclass__`` and are discovered by the | ||
| orchestrator through ``NavTechnique._registry``. Each | ||
| technique declares its accepted feature types, whether it requires a pass-1 prior, and the |
There was a problem hiding this comment.
Cross-reference the registry attribute explicitly.
NavTechnique._registry is a narrative API mention and should be written with an attribute role (for example, :attr:\~nav.nav_technique.nav_technique.NavTechnique._registry``).
As per coding guidelines, “Do not use bare CamelCase or module.symbol text for API symbols in narrative prose, even when wrapped in inline literals”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques.rst` around lines 13 - 15, Replace the
bare narrative mention of NavTechnique._registry with an explicit Sphinx
attribute role; locate the sentence referencing NavTechnique._registry in the
docs and change the inline literal to use the attr role (for example
:attr:`~nav.nav_technique.nav_technique.NavTechnique._registry`) so the registry
attribute is cross-referenced correctly in the rendered docs.
Comment on lines
+129
to
+131
| * ``--nav-models LIST``: a comma-separated glob-pattern list selecting which | ||
| ``NavModel`` instances run. Names follow the ``stars`` / | ||
| ``body:NAME`` / ``rings:PLANET`` convention. Defaults to ``*``. See |
There was a problem hiding this comment.
Replace bare NavModel / NavTechnique mentions with Sphinx class references.
In these updated CLI option descriptions, the API class names are in inline literals instead of :class: links. Please convert them to maintain consistency and strict docs cross-reference hygiene.
As per coding guidelines: "Do not use bare CamelCase or module.symbol text for API symbols in narrative prose, even when wrapped in inline literals".
Also applies to: 136-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user_guide_navigation.rst` around lines 129 - 131, Replace the bare
inline literals "NavModel" and "NavTechnique" with proper Sphinx cross-reference
class roles (e.g. use :class: references pointing to the API classes) so the CLI
option descriptions use :class:`...` links instead of plain CamelCase; update
the occurrences in the shown block (the ``--nav-models LIST`` line) and the
similar mentions at lines 136–138 to reference the actual fully-qualified class
symbols (the NavModel and NavTechnique classes) so the documentation builds with
correct cross-references.
Comment on lines
+454
to
+455
| ``NavModel._registry`` via ``__init_subclass__`` so that | ||
| :func:`~nav.nav_model.nav_model.build_models_for_obs` discovers it. Public surface |
There was a problem hiding this comment.
Replace bare/inline-literal API symbols with Sphinx cross-reference roles
A few API mentions are still plain literals instead of role-based links (NavModel._registry, __init_subclass__, _compute_limb_mask_from_body_mask, _create_annotations, LIMB_ARC). Please convert these to :attr: / :meth: / enum-member references for consistency and to satisfy docs policy.
Suggested patch
-The class registers itself in ``NavModel._registry`` via ``__init_subclass__`` so that
+The class registers itself in
+:attr:`~nav.nav_model.nav_model.NavModel._registry` via
+:meth:`~nav.nav_model.nav_model.NavModel.__init_subclass__` so that
:func:`~nav.nav_model.nav_model.build_models_for_obs` discovers it.
@@
-- ``_compute_limb_mask_from_body_mask`` — computes the limb mask from the body mask via
+- :meth:`~nav.nav_model.nav_model_body_base.NavModelBodyBase._compute_limb_mask_from_body_mask` —
+ computes the limb mask from the body mask via
discrete neighbour shifts. Used by the simulated body model.
-- ``_create_annotations`` — builds the body-label
+- :meth:`~nav.nav_model.nav_model_body_base.NavModelBodyBase._create_annotations` —
+ builds the body-label
:class:`~nav.annotation.annotations.Annotations` collection for the summary PNG. Consumes
@@
- :data:`~nav.feature.feature_type.NavFeatureType.BODY_BLOB` feature instead of a LIMB_ARC.
+ :data:`~nav.feature.feature_type.NavFeatureType.BODY_BLOB` feature instead of an
+ :attr:`~nav.feature.feature_type.NavFeatureType.LIMB_ARC`.As per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use appropriate Sphinx cross-reference roles" and "Do not use bare CamelCase or module.symbol text for API symbols in narrative prose, even when wrapped in inline literals."
Also applies to: 516-520, 631-631
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_navigation_models_body.rst` around lines 454 - 455,
Replace the bare inline-literal API mentions with proper Sphinx cross-reference
roles: change ``NavModel._registry`` to :attr:`NavModel._registry`,
``__init_subclass__`` to :meth:`__init_subclass__` (or
:meth:`NavModel.__init_subclass__` for clarity), the reference to
:func:`~nav.nav_model.nav_model.build_models_for_obs` is fine but ensure
consistency, and convert ``_compute_limb_mask_from_body_mask``,
``_create_annotations``, and ``LIMB_ARC`` to the appropriate roles (:func: or
:meth: for private functions/methods like
:meth:`_compute_limb_mask_from_body_mask` / :meth:`_create_annotations`, and
:data:`LIMB_ARC` or :attr:`LIMB_ARC` for the constant/enum member) so all API
symbols use Sphinx cross-reference roles consistently throughout the document.
| Examples | ||
| ======== | ||
|
|
||
| **Two agreeing techniques.** Pass 1 produces |
There was a problem hiding this comment.
Use a single space after period in example lead-ins
These three lines use double spaces after the sentence-ending period; repo docs style requires exactly one.
Suggested patch
-**Two agreeing techniques.** Pass 1 produces
+**Two agreeing techniques.** Pass 1 produces
@@
-**Single-link grouping with three techniques.** Three techniques converge:
+**Single-link grouping with three techniques.** Three techniques converge:
@@
-**Rank-deficient ring-edge fit.** A flat-ring-only scene produces a
+**Rank-deficient ring-edge fit.** A flat-ring-only scene produces aAs per coding guidelines: "Always use one space between the period at the end of a sentence and the next sentence."
Also applies to: 177-177, 186-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_orchestrator_ensemble.rst` at line 167, The example
lead-ins in the docs use two spaces after the sentence-ending period; update
each affected sentence (e.g., "Two agreeing techniques. Pass 1 produces" and
the similar lines referenced later) to use a single space after the period so
they read "Two agreeing techniques. Pass 1 produces" (and analogous fixes for
the other example sentences), ensuring the repo docs style of exactly one space
between sentences.
Five real bugs surfaced by review of the dev_guide pages: - Drop the :meth: role on Config._load_yaml — private members are not in autodoc, so the reference does not resolve. - Use :attr: for NavResult.status_reason (was :class:). - Use :class: for NavFilterKind (was :data:; it is an Enum class). - Rephrase the simulated-ring "uses carries" sentence so the RingFeature data model is the subject of a single clear clause. - Use :data: for NavFeatureType.RING_ANNULUS to match the rest of the simulated-ring page. - Convert ``BodyMosaic`` / ``RingMosaic`` mentions in the familiarization plan to :class: cross-references. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
docs/dev_guide/dev_guide_navigation_models_ring_simulated.rst (1)
58-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a Sphinx data cross-reference for
RING_ANNULUSin configuration prose.Line 59 uses a bare inline literal for a data constant. Replace
RING_ANNULUSwith:data:\~nav.feature.feature_type.NavFeatureType.RING_ANNULUS`` to keep role usage consistent and avoid nitpicky doc-reference drift.As per coding guidelines, “Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/dev_guide/dev_guide_navigation_models_ring_simulated.rst` around lines 58 - 60, Replace the bare inline literal RING_ANNULUS used in the prose describing the feature_type field with a Sphinx data cross-reference; specifically change the mention of RING_ANNULUS to :data:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS` so references to the NavFeatureType data constant are consistent and follow the docs rule for cross-referencing attributes/constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/dev_guide/dev_guide_config_and_static_data.rst`:
- Around line 250-251: The first sentence token "1. **Cite only documents
fetched in-session.** Every citation must be" contains two spaces after the
period; change it to a single space so it reads "1. **Cite only documents
fetched in-session.** Every citation must be" ensuring the doc string shown (the
sentence starting with "1. **Cite only documents fetched in-session.**") uses
one space after the period per the documentation spacing guideline.
- Around line 226-227: Replace the inline literal ``Config`` in the prose with a
Sphinx class cross-reference role—e.g., use :class:`Config` (or a fully
qualified name like :class:`mypackage.module.Config` if needed) so the reference
resolves correctly; update the string in the sentence that currently reads
"parsed ``Config``" to use the :class: role and verify the cross-reference
builds in the docs.
- Around line 296-297: Replace the bare inline literal reference to
Config._load_yaml with a proper Sphinx cross-reference role or rewrite the
phrase to avoid the method name; specifically, either change the inline literal
to the method role :py:meth:`Config._load_yaml` (or the appropriate domain/role
used in this docs set) or rephrase the sentence to refer to "the YAML loader" so
it conforms to the rule that every mention of a method must use a
cross-reference role.
In `@docs/dev_guide/dev_guide_familiarization.rst`:
- Around line 272-273: Update the prose to use a Sphinx module cross-reference
instead of an inline literal: replace the backticked ``_star_helpers`` mention
with the :mod:`_star_helpers` role in the sentence that reads "The shared
``_star_helpers`` module is internal and not part of the autodoc API." so it
becomes "The shared :mod:`_star_helpers` module is internal and not part of the
autodoc API."; ensure no other inline-literal mentions of _star_helpers remain
in that sentence.
- Line 325: Replace the bare inline mention of the module with a Sphinx module
cross-reference: change the text "nav.ui.common" to use the module role
:mod:`nav.ui.common` so the reference is treated as a proper Sphinx
cross-reference and prevents autodoc/cross-ref warnings.
- Around line 63-64: Replace the bare inline literals with Sphinx
cross-reference roles: change the prose that reads "in-memory ``Observation``
(from ``oops``)" to use the appropriate roles for the class and module (e.g. use
a class role like :class:`oops.Observation` or :class:`~oops.Observation` and a
module role like :mod:`oops`) so the mention of Observation and oops uses Sphinx
cross-references instead of raw inline code.
In `@docs/dev_guide/dev_guide_logging.rst`:
- Line 67: Replace the bare reference "pdslogger" in the prose with the
appropriate Sphinx module role so it becomes a cross-reference (e.g., use
:mod:`pdslogger`); update the text in docs/dev_guide/dev_guide_logging.rst where
"pdslogger" appears in narrative prose to use the module role so the reference
is role-consistent with other API mentions.
In `@docs/dev_guide/dev_guide_techniques_image_derivatives.rst`:
- Line 67: Replace all occurrences of two spaces after sentence-ending periods
with a single space in the documentation; specifically update the sentence
starting "Directional non-maximum suppression. Each candidate pixel is kept
only if its" (and the other similar sentences flagged in the comment) so there
is one space after the period, ensuring all sentences in
dev_guide_techniques_image_derivatives.rst use a single space between sentences.
- Around line 153-154: Replace the inline literal mention of the dataclass
method "__post_init__" with a Sphinx method cross-reference using
:meth:`__post_init__`; update the sentence so it reads something like "The
dataclass's :meth:`__post_init__` rejects..." ensuring the cross-reference role
is used and punctuation/quotes remain consistent (edit the line that currently
contains "__post_init__" to use :meth:`__post_init__`).
- Around line 221-227: Update the narrative prose to use Sphinx :mod:
cross-reference roles for module mentions: replace plain "numpy" with
":mod:`numpy`" and plain "SciPy" with ":mod:`scipy`"; also ensure any
configuration variable mentioned in prose like "image_gradient_sigma_px" is
cross-referenced with the appropriate role (e.g. :data:, :confval:, or :ref: as
per project convention) so all module and symbol mentions in the paragraph
around image_gradient_sigma_px, numpy, and SciPy use the proper Sphinx roles.
---
Duplicate comments:
In `@docs/dev_guide/dev_guide_navigation_models_ring_simulated.rst`:
- Around line 58-60: Replace the bare inline literal RING_ANNULUS used in the
prose describing the feature_type field with a Sphinx data cross-reference;
specifically change the mention of RING_ANNULUS to
:data:`~nav.feature.feature_type.NavFeatureType.RING_ANNULUS` so references to
the NavFeatureType data constant are consistent and follow the docs rule for
cross-referencing attributes/constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f399457-9f3f-42d0-9484-3d180eb819c2
📒 Files selected for processing (5)
docs/dev_guide/dev_guide_config_and_static_data.rstdocs/dev_guide/dev_guide_familiarization.rstdocs/dev_guide/dev_guide_logging.rstdocs/dev_guide/dev_guide_navigation_models_ring_simulated.rstdocs/dev_guide/dev_guide_techniques_image_derivatives.rst
Comment on lines
+226
to
+227
| are stripped at config-load time so the documentation does not bloat the | ||
| parsed ``Config`` — the citation lives in the file for human review only. |
There was a problem hiding this comment.
Use a class cross-reference role for Config in prose.
Config is currently inline-literal text in narrative prose; use a class role instead.
Suggested fix
-parsed ``Config`` — the citation lives in the file for human review only.
+parsed :class:`~nav.config.config.Config` — the citation lives in the file for human review only.As per coding guidelines: “Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_config_and_static_data.rst` around lines 226 - 227,
Replace the inline literal ``Config`` in the prose with a Sphinx class
cross-reference role—e.g., use :class:`Config` (or a fully qualified name like
:class:`mypackage.module.Config` if needed) so the reference resolves correctly;
update the string in the sentence that currently reads "parsed ``Config``" to
use the :class: role and verify the cross-reference builds in the docs.
Comment on lines
+250
to
+251
| 1. **Cite only documents fetched in-session.** Every citation must be | ||
| traceable to a ``WebFetch`` / ``WebSearch`` lookup performed in the same |
There was a problem hiding this comment.
Fix sentence spacing to one space after period.
There are two spaces after the period before “Every citation”.
Suggested fix
-1. **Cite only documents fetched in-session.** Every citation must be
+1. **Cite only documents fetched in-session.** Every citation must beAs per coding guidelines: “Always use one space between the period at the end of a sentence and the next sentence in documentation.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. **Cite only documents fetched in-session.** Every citation must be | |
| traceable to a ``WebFetch`` / ``WebSearch`` lookup performed in the same | |
| 1. **Cite only documents fetched in-session.** Every citation must be | |
| traceable to a ``WebFetch`` / ``WebSearch`` lookup performed in the same |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_config_and_static_data.rst` around lines 250 - 251,
The first sentence token "1. **Cite only documents fetched in-session.** Every
citation must be" contains two spaces after the period; change it to a single
space so it reads "1. **Cite only documents fetched in-session.** Every citation
must be" ensuring the doc string shown (the sentence starting with "1. **Cite
only documents fetched in-session.**") uses one space after the period per the
documentation spacing guideline.
Comment on lines
+296
to
+297
| ``Config._load_yaml`` strips every mapping key whose | ||
| name starts with ``_`` before merging, so ``_sources`` blocks never appear in |
There was a problem hiding this comment.
Avoid bare method-name prose or convert to a proper method cross-reference.
Config._load_yaml is referenced as inline literal in narrative prose, which violates the symbol-role requirement. Either use a resolvable method role or rewrite to avoid naming the method directly (e.g., “the YAML loader”).
Suggested fix (role-compliant wording without method symbol)
-``Config._load_yaml`` strips every mapping key whose
+The YAML loader strips every mapping key whoseAs per coding guidelines: “Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_config_and_static_data.rst` around lines 296 - 297,
Replace the bare inline literal reference to Config._load_yaml with a proper
Sphinx cross-reference role or rewrite the phrase to avoid the method name;
specifically, either change the inline literal to the method role
:py:meth:`Config._load_yaml` (or the appropriate domain/role used in this docs
set) or rephrase the sentence to refer to "the YAML loader" so it conforms to
the rule that every mention of a method must use a cross-reference role.
Comment on lines
+63
to
+64
| in-memory ``Observation`` (from ``oops``) carrying the per-pixel features | ||
| that the rest of the pipeline consumes. |
There was a problem hiding this comment.
Use Sphinx roles for Observation and oops in prose.
Observation and oops are code entities written as bare inline literals; this violates the doc cross-reference rule and can cause strict Sphinx/nitpicky warnings.
Suggested patch
-Goal: understand how an image goes from a file path on disk to an
-in-memory ``Observation`` (from ``oops``) carrying the per-pixel features
+Goal: understand how an image goes from a file path on disk to an
+in-memory :class:`~oops.observation.Observation` (from :mod:`oops`) carrying the per-pixel features
that the rest of the pipeline consumes.As per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| in-memory ``Observation`` (from ``oops``) carrying the per-pixel features | |
| that the rest of the pipeline consumes. | |
| in-memory :class:`~oops.observation.Observation` (from :mod:`oops`) carrying the per-pixel features | |
| that the rest of the pipeline consumes. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_familiarization.rst` around lines 63 - 64, Replace
the bare inline literals with Sphinx cross-reference roles: change the prose
that reads "in-memory ``Observation`` (from ``oops``)" to use the appropriate
roles for the class and module (e.g. use a class role like
:class:`oops.Observation` or :class:`~oops.Observation` and a module role like
:mod:`oops`) so the mention of Observation and oops uses Sphinx cross-references
instead of raw inline code.
Comment on lines
+272
to
+273
| - The shared ``_star_helpers`` module is internal and not part of the | ||
| autodoc API. |
There was a problem hiding this comment.
Convert _star_helpers module mention to a :mod: cross-reference.
This is a module mention in narrative prose but currently uses an inline literal, which violates the Sphinx-role guideline.
Suggested patch
-- The shared ``_star_helpers`` module is internal and not part of the
+- The shared :mod:`nav.nav_technique._star_helpers` module is internal and not part of the
autodoc API.As per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - The shared ``_star_helpers`` module is internal and not part of the | |
| autodoc API. | |
| - The shared :mod:`nav.nav_technique._star_helpers` module is internal and not part of the | |
| autodoc API. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_familiarization.rst` around lines 272 - 273, Update
the prose to use a Sphinx module cross-reference instead of an inline literal:
replace the backticked ``_star_helpers`` mention with the :mod:`_star_helpers`
role in the sentence that reads "The shared ``_star_helpers`` module is internal
and not part of the autodoc API." so it becomes "The shared :mod:`_star_helpers`
module is internal and not part of the autodoc API."; ensure no other
inline-literal mentions of _star_helpers remain in that sentence.
| - :mod:`nav.nav_technique.nav_technique_manual` | ||
| - :mod:`nav.ui.manual_nav_dialog` | ||
| - :mod:`nav.ui.library_entry` | ||
| - The ``nav.ui.common`` module is internal and not autodoc'd. |
There was a problem hiding this comment.
Use :mod: for nav.ui.common module mention.
nav.ui.common is referenced as bare inline text in prose; it should use a module cross-reference role for consistency and strict-doc compliance.
Suggested patch
-- The ``nav.ui.common`` module is internal and not autodoc'd.
+- The :mod:`nav.ui.common` module is internal and not autodoc'd.Based on learnings: "When a class, method, function, module, attribute, or data constant is added, removed, or renamed, update every cross-reference to it across the docs tree in the same change. Validate by building with sphinx-build -W -b html AND sphinx-build -n -b html; both MUST succeed with zero warnings."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - The ``nav.ui.common`` module is internal and not autodoc'd. | |
| - The :mod:`nav.ui.common` module is internal and not autodoc'd. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_familiarization.rst` at line 325, Replace the bare
inline mention of the module with a Sphinx module cross-reference: change the
text "nav.ui.common" to use the module role :mod:`nav.ui.common` so the
reference is treated as a proper Sphinx cross-reference and prevents
autodoc/cross-ref warnings.
| Log levels | ||
| ========== | ||
|
|
||
| pdslogger exposes the standard six-level ladder. Pick the level that matches the |
There was a problem hiding this comment.
Use a Sphinx module role for pdslogger in narrative prose.
Line 67 references a module as bare text. Please use a module cross-reference role to keep docs role-consistent.
Suggested fix
-pdslogger exposes the standard six-level ladder. Pick the level that matches the
+:mod:`pdslogger` exposes the standard six-level ladder. Pick the level that matches theAs per coding guidelines: “Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pdslogger exposes the standard six-level ladder. Pick the level that matches the | |
| :mod:`pdslogger` exposes the standard six-level ladder. Pick the level that matches the |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_logging.rst` at line 67, Replace the bare reference
"pdslogger" in the prose with the appropriate Sphinx module role so it becomes a
cross-reference (e.g., use :mod:`pdslogger`); update the text in
docs/dev_guide/dev_guide_logging.rst where "pdslogger" appears in narrative
prose to use the module role so the reference is role-consistent with other API
mentions.
| default keeps single-pixel noise excursions out of the DT input while admitting limb, | ||
| terminator, and ring edges with margin. | ||
|
|
||
| - **Directional non-maximum suppression.** Each candidate pixel is kept only if its |
There was a problem hiding this comment.
Normalize sentence spacing to one space.
These lines use two spaces between sentences (e.g., after a period before the next sentence). Please normalize to one space.
As per coding guidelines: "Always use one space between the period at the end of a sentence and the next sentence in documentation."
Also applies to: 224-224, 234-234, 241-241, 249-249
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques_image_derivatives.rst` at line 67,
Replace all occurrences of two spaces after sentence-ending periods with a
single space in the documentation; specifically update the sentence starting
"Directional non-maximum suppression. Each candidate pixel is kept only if its"
(and the other similar sentences flagged in the comment) so there is one space
after the period, ensuring all sentences in
dev_guide_techniques_image_derivatives.rst use a single space between sentences.
Comment on lines
+153
to
+154
| The dataclass's ``__post_init__`` rejects any non-positive or non-finite field with | ||
| :exc:`ValueError`; a malformed override fails fast at construction rather than mid-image. |
There was a problem hiding this comment.
Use a method cross-reference for __post_init__.
Line 153 mentions a specific method in narrative prose but uses inline literal formatting. Use :meth: for consistency and nitpicky Sphinx correctness.
Suggested fix
-The dataclass's ``__post_init__`` rejects any non-positive or non-finite field with
+The dataclass's :meth:`~nav.nav_orchestrator.image_derivatives.ImageDerivativesConfig.__post_init__` rejects any non-positive or non-finite field withAs per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role (:class:, :meth:, :func:, :mod:, :attr:, :data:)..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques_image_derivatives.rst` around lines 153 -
154, Replace the inline literal mention of the dataclass method "__post_init__"
with a Sphinx method cross-reference using :meth:`__post_init__`; update the
sentence so it reads something like "The dataclass's :meth:`__post_init__`
rejects..." ensuring the cross-reference role is used and punctuation/quotes
remain consistent (edit the line that currently contains "__post_init__" to use
:meth:`__post_init__`).
Comment on lines
+221
to
+227
| The image-derivatives helpers operate on numpy arrays rather than on observations; the | ||
| worked examples below are numerical illustrations rather than image-library scenes. | ||
|
|
||
| **One-pass cost on a typical extended-FOV image.** A 1024 × 1024 extended-FOV image at the | ||
| default ``image_gradient_sigma_px = 1.2`` runs one separable Gaussian (truncated by SciPy's | ||
| default at four sigma, ~9 × 9 effective kernel) plus two separable Sobel passes — three | ||
| passes over the image, no full-image FFT. Reusing the |
There was a problem hiding this comment.
Cross-reference external modules with :mod: roles.
Line 221 (numpy) and Line 225 (SciPy) are module mentions in narrative prose and should use :mod: roles.
Suggested fix
-The image-derivatives helpers operate on numpy arrays rather than on observations; the
+The image-derivatives helpers operate on :mod:`numpy` arrays rather than on observations; the
...
-default at four sigma, ~9 × 9 effective kernel) plus two separable Sobel passes — three
+default at four sigma, ~9 × 9 effective kernel) plus two separable Sobel passes from :mod:`scipy` — threeAs per coding guidelines: "Every mention of a class, method, function, module, attribute, or data constant in narrative prose MUST use the appropriate Sphinx cross-reference role..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/dev_guide/dev_guide_techniques_image_derivatives.rst` around lines 221 -
227, Update the narrative prose to use Sphinx :mod: cross-reference roles for
module mentions: replace plain "numpy" with ":mod:`numpy`" and plain "SciPy"
with ":mod:`scipy`"; also ensure any configuration variable mentioned in prose
like "image_gradient_sigma_px" is cross-referenced with the appropriate role
(e.g. :data:, :confval:, or :ref: as per project convention) so all module and
symbol mentions in the paragraph around image_gradient_sigma_px, numpy, and
SciPy use the proper Sphinx roles.
Three targeted changes to make Sphinx nitpicky mode reach more cross-references cleanly: - Add package-level ``automodule:: nav.<package>`` to the api_reference pages so ``:mod:\`nav.config\``` etc. resolve. Each package __init__.py already carries a docstring worth exposing. - Add intersphinx for filecache and pdslogger (both ship Sphinx docs on RTD) so cross-package references resolve to upstream pages. - Add ``nitpick_ignore_regex`` for symbol categories that have no inventory we can link to: oops (no Sphinx site), test modules, sibling packages outside the nav API surface, typing internals (numpy._typing, argparse._*), and TypeVars / type aliases that Sphinx does not register as cross-reference targets. Net effect on this PR: 370 → 198 nitpicky warnings; ``sphinx-build -W -b html`` continues to build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rf_core_rewrite(which carries phases 0–9); merges only the docs work into that integration branch.docs/developer_guide*.rstintodocs/dev_guide/with thedev_guide_<xxx>prefix, retargets every internal:doc:cross-reference, and rewrites/reorganizes the developer guide to track the post-rewrite architecture.docs/dev_guide/dev_guide_familiarization.rst) sequencing the docs andsrc/navmodules a new contributor should read in ten stages — orientation, foundations, inputs, a first end-to-end pipeline (NavModelBody+BodyLimbNav), the orchestrator round-trip, the remaining models paired with their techniques (stars → bodies → rings → titan, then manual), simulated images, calibration / regression, downstream products (reprojection, backplanes, PDS4), and finally extending and conventions. Every class, method, function, and module reference is a Sphinx cross-reference into the autodoc API.Test plan
./scripts/run-all-checks.shpasses (ruff + mypy + pytest + sphinx + pymarkdown).sphinx-build -W -b html docs docs/_buildbuilds clean.sphinx-build -b linkcheck docs docs/_buildreports no broken internal links.dev_guide_familiarization.html: every class / module hyperlink lands on the correct API page, and each stage's bullet structure renders as nested sub-bullets.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Bug Fixes