Skip to content

Harden registry indexing: model type fallback and training scripts stale delete - #5456

Merged
Jeremy Rose (jeremyrose-viam) merged 3 commits into
viamrobotics:mainfrom
jeremyrose-viam:fix-registry-indexing-docs-5032
Oct 23, 2026
Merged

Jeremy Rose (jeremyrose-viam) merged 3 commits into
viamrobotics:mainfrom
jeremyrose-viam:fix-registry-indexing-docs-5032

Conversation

@jeremyrose-viam

@jeremyrose-viam Jeremy Rose (jeremyrose-viam) commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Two fixes in .github/workflows/get_modular_resources.py, the script behind the upsert-modular-resources job in docs-publish. Tracked in DOCS-5032, following the framework fix.

Changes

  • Unknown model type no longer raises. model_type_to_str logs a warning and returns "" for an unrecognized ModelType, the same handling model_framework_to_str has. Before, a new enum value stopped the job partway through ML model indexing and skipped the remaining models, all training scripts, and stale-document cleanup.
  • Training scripts cleanup now runs. The trainingscripts delete built its filter as 'last_updated: <' + time_now with an int, which raised a TypeError that the surrounding except swallowed. It now uses str(time_now) like the resources and mlmodels deletes. Registry items that were deleted or made private now drop out of the training scripts index.

Effect of the first working delete

I compared the public trainingscripts index with the registry's current list (ListRegistryItems, 6 public training scripts). The index holds 8 documents. The 6 live ones were re-stamped on the latest run. The other 2 are lillians/training24 and lillians/training25, last updated 2026-03-05 and absent from the registry. Those 2 are what the first run after merge should delete. The "Training scripts deleted" line in the first docs-publish run after merge should show them.

Testing

  • The script compiles, and model_type_to_str returns the expected labels for 0 to 3 and the fallback for an unknown value.
  • No end-to-end run: it needs CI secrets (the Viam API key and Typesense admin key).

Open question

The script calls ListRegistryItems once, and the response has no next-page token (the request has page_token). The call returned 847 items in a single response, which doesn't look like a cap, but whether the server ever limits results isn't confirmed. A working delete treats anything missing from that list as deleted, so a capped response would drop real training scripts until the next run. mlmodels and resources already have this exposure. I'd like the registry or app team to confirm before or shortly after merge.

🤖 Generated with Claude Code

…ale delete

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@netlify

netlify Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for viam-docs ready!

Name Link
🔨 Latest commit 529ddd5
🔍 Latest deploy log https://app.netlify.com/projects/viam-docs/deploys/6adbb813af41373f2947ac49
😎 Deploy Preview https://deploy-preview-5456--viam-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 38 (🔴 down 3 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Oct 22, 2026
@elizafarley

Copy link
Copy Markdown
Contributor

Docs review: Ready to merge

This PR makes two fixes in get_modular_resources.py. An unknown ModelType value now logs a warning and indexes as "" instead of raising. The trainingscripts stale-document delete now builds its filter with str(time_now). Both fixes are correct and small, and the server code answers the PR's open question about pagination (see below).

CI: All required checks passing

Docs health: No regressions (no docs/ pages changed)

The open question: ListRegistryItems is not capped

The server doesn't limit results, so a working stale delete won't drop live training scripts because of a short response. In viamrobotics/app:

  • domains/service/registry_item.go:497: ListRegistryItems returns everything db.RegistryItemList returns, with no slicing.
  • data/registry_item.go:1778: RegistryItemList runs registryItemCollection.Find(ctx, query) with no FindOptions, so no limit is set. The script sends no search_term, so it takes the plain Find path at about line 2088. The Atlas Search branch isn't reached.
  • filter.PageToken is copied from the request (data/registry_item_proto.go:157) but never read in data/registry_item.go, so the field is a no-op.

Note: this is true of main today. If the app team ever adds real paging, this script, the mlmodels delete, and the resources delete would all need to loop over pages. A one-line comment at the ListRegistryItems call would help a future reader.

Nits

  • .github/workflows/get_modular_resources.py:34 (and :53): a plain print("WARNING: ...") is easy to miss in a passing docs-publish log. Fix: print ::warning::unknown model type ... instead, so GitHub Actions shows it as an annotation on the run summary.
  • .github/workflows/get_modular_resources.py:174-186: this predates the PR, but the three deletes share one try. A failure in the resources delete still skips the other two, and the except only prints. Fix (optional, could be a follow-up): give each delete its own try, or re-raise after logging so the job fails visibly.
Verified against source (5 claims)
  • model_type_map covers every current ModelType value (0 to 3): viamrobotics/api proto/viam/app/mltraining/v1/ml_training.proto:50-53
  • model_framework_map covers every current ModelFramework value (0 to 5), so the fallback matches it: viamrobotics/api proto/viam/app/mltraining/v1/ml_training.proto:57-65
  • time_now is an int (int(time.time()), line 69), so '...<' + time_now raised TypeError, and the surrounding except swallowed it (lines 174-186)
  • "" is already what the script indexes for a model with no type (lines 137-140), so the fallback adds no new value to the type facet
  • ListRegistryItemsResponse has no next-page token: viamrobotics/api proto/viam/app/v1/app.proto:1738-1740. The server doesn't cap results: viamrobotics/app data/registry_item.go:1778, ~2088

Automated review by Claude. Rerunning the review updates this comment.

@jeremyrose-viam
Jeremy Rose (jeremyrose-viam) merged commit 8142af5 into viamrobotics:main Oct 23, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to build This pull request is marked safe to build from a trusted zone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants