Coverage: guard missing proto overrides + remove navigation (#5126)#5143
Open
btshrewsbury-viam wants to merge 1 commit into
Open
Coverage: guard missing proto overrides + remove navigation (#5126)#5143btshrewsbury-viam wants to merge 1 commit into
btshrewsbury-viam wants to merge 1 commit into
Conversation
Two of the crashes that were stacked behind the gripper KeyError (#5139), both surfaced once that one was fixed: (a) Missing proto-description override files crashed the generator. Proto descriptions come from override files (static/include/.../overrides/protos/ {resource}.{proto}.md), but the description-reading open() was not guarded by the os.path.isfile check the content-writing branch already uses. Guard it and fall back to a blank description so a missing override reports gracefully instead of crashing (same spirit as #5139). (b) Fully remove navigation. #5067 removed the navigation docs section (redirected to motion-planning) and deleted its override files, but left navigation in the CSV and in the generator's services list, so coverage still tried to document it and crashed on the deleted navigation.GetMode.md override. Remove navigation from sdk_protos_map.csv, the `services` list, the proto-URL map, and the service anchor logic. (Removing only the CSV rows would leave navigation still scraped and flag its methods as "unused"; dropping it from the services list stops that.) Verified by running the generator locally: it now completes with no crash (previously KeyError then FileNotFoundError), and navigation no longer appears. Note: this does not make check-methods green -- with the crashes gone, the check now reports ~110 pre-existing "unused" warnings it had been masking (SDK methods not mapped in the CSV). Each is a separate document-vs-ignore decision. Refs #5126
✅ Deploy Preview for viam-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
viambot
added
the
safe to build
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
2 participants
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.
Fixes two of the crashes stacked behind the gripper
KeyError(#5139), both surfaced once that one was fixed.(a) Guard missing proto-description overrides
Proto descriptions come from override files (
static/include/.../overrides/protos/{resource}.{proto}.md), but the description-readingopen()wasn't guarded by theos.path.isfilecheck the content-writing branch already uses. Guard it and fall back to a blank description with aWARNING(so a genuinely-missing override — an authoring gap — is still surfaced, not silently blanked). Same spirit as #5139.(b) Fully remove navigation
#5067 removed the navigation docs section (redirect to motion-planning) and deleted its override files, but left navigation in
sdk_protos_map.csvand the generator'sserviceslist — so coverage still tried to document it and crashed on the deletednavigation.GetMode.md. Removed navigation from the CSV, theserviceslist, the proto-URL map, and the service anchor logic. (Removing only the CSV rows would leave navigation still scraped and flag its methods as "unused"; dropping it fromservicesstops that.)Verification
Ran the generator locally: completes with no crash (was
KeyErrorthenFileNotFoundError), and navigation no longer appears. Dispatching check-methods on this branch to confirm in CI (Python 3.9) — grading on the coverage step log (job is always green viacontinue-on-error).Honest scope — does NOT close #5126
With the crashes gone, the check now reports ~110 pre-existing "unused" warnings it had been masking (SDK methods not mapped in the CSV). Those are the real remaining work — each a document-vs-ignore decision — and are handled separately, not silenced to force green.
Refs #5126
🤖 Generated with Claude Code