Skip to content

refactor(commands): extract shared embedding core - #1071

Merged
lfnovo merged 1 commit into
mainfrom
refactor/embedding-command-core
Jul 11, 2026
Merged

refactor(commands): extract shared embedding core#1071
lfnovo merged 1 commit into
mainfrom
refactor/embedding-command-core

Conversation

@lfnovo

@lfnovo lfnovo commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

Deduplicates commands/embedding_commands.py (and the thrice-copied full_model_dump). Pure refactor — behavior-identical: same outputs, same success=False paths, same log messages, same retry configuration.

  • Shared embed core: embed_note / embed_insight / embed_source were structural clones (load → validate → embed → UPSERT + an identical ~20-line ValueError/Exception epilogue ×3). They now run through one _embed_record(...) runner holding the epilogue; note and insight additionally share _embed_markdown_record(...) for their identical bodies.
  • Rebuild loop unified: rebuild_embeddings_command submitted jobs via three copy-pasted loops — now one _submit_embedding_jobs(kind, command_name, id_field, ids) helper (same progress/error log messages).
  • Shared retry config: the four embed-family commands reuse a single EMBED_RETRY_CONFIG dict. Added a # NOTE: marking that stop_on: [ValueError, ...] can never trigger today (commands catch ValueError internally and return success=False) — intentionally preserved as-is for a future error-handling PR.
  • full_model_dump(): was copy-pasted in embedding_commands.py, podcast_commands.py and source_commands.py, but only podcast_commands actually used it. Moved to open_notebook/utils/model_utils.py (exported from open_notebook.utils), imported in podcast_commands; the two dead copies were removed rather than re-imported (F401 is enforced again as of chore(lint): re-enable F401/F841/E722 and fix fallout #1062).

Line delta: +232 / −319 overall; embedding_commands.py alone drops from 799 to ~636 lines.

Verification

  • uv run pytest tests/ — 403 passed
  • ruff check . — clean; ruff format — clean
  • uv run python -m mypy commands/ open_notebook/utils/ — 8 pre-existing errors, identical count and locations as main (none in touched lines)
  • Import smoke test: import commands registers all 8 commands with retry configs identical to main (verified via the surreal-commands registry)

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Re-trigger cubic

- embed_note / embed_insight / embed_source now share a single
  load->embed->write runner (_embed_record) with one common
  ValueError/Exception epilogue; note and insight additionally share
  _embed_markdown_record for the identical load/validate/embed/UPSERT body
- rebuild_embeddings submits jobs for sources/notes/insights through one
  _submit_embedding_jobs helper instead of three copy-pasted loops
- the four embed-family commands reuse one EMBED_RETRY_CONFIG dict, with a
  NOTE marking that stop_on can never trigger today (commands catch
  ValueError internally) - preserved as-is for a future error-handling PR
- full_model_dump() was copy-pasted in three command files but only used by
  podcast_commands: moved to open_notebook/utils/model_utils.py, imported
  where used, dead copies in embedding/source commands removed

Behavior-identical: same outputs, success=False paths, log messages and
retry configuration. 403 tests pass; ruff and mypy clean (no new errors).
@lfnovo
lfnovo force-pushed the refactor/embedding-command-core branch from 19b26ad to 0d60c68 Compare July 11, 2026 21:56
@lfnovo
lfnovo merged commit 038c9fa into main Jul 11, 2026
10 checks passed
@lfnovo
lfnovo deleted the refactor/embedding-command-core branch September 2, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant