Skip to content

Conversation

@TomMelt
Copy link

@TomMelt TomMelt commented Oct 16, 2025

Pull Request

Summary

fixes #803

Changes Made

  • Add additional check for embedding models of size 384 to the files document_storage_service.py and code_storage_service.py

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

I no longer get any warnings in the logs about the embedding model size not being supported.
Furthermore, it now correctly uploads my files.

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

Additional Notes

Summary by CodeRabbit

  • New Features

    • Added support for an additional embedding dimension (384) across storage operations, improving compatibility with more embedding formats.
    • Preserves existing handling for previously supported embedding sizes to avoid regressions.
  • Bug Fixes

    • Records with supported dimensions are now correctly recognized, reducing skipped entries due to unrecognized embedding sizes.

coleam00 and others added 10 commits October 11, 2025 07:16
0.1.0 release of Archon! Improvements across the board to crawling speed and reliability, RAG strategies, docs, task management, and documentation.
Release workflow update for Archon 0.1.0 release.
Adjusting permissions in release note workflow for Archon 0.1.0 release
Embedding models of size 384 are supported in the sql files, but
`embedding_384` was missing from the python file
`document_storage_service.py`.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

Added explicit handling for 384-dimensional embeddings in storage services; both document and code storage now map embedding_dim == 384 to the embedding_384 DB column, while preserving existing mappings for 768, 1024, 1536, and 3072 dimensions.

Changes

Cohort / File(s) Change Summary
Embedding dimension support
python/src/server/services/storage/document_storage_service.py, python/src/server/services/storage/code_storage_service.py
Added explicit conditional for embedding_dim == 384 to select embedding_384 column; preserved existing branches for 768, 1024, 1536, and 3072; unchanged behavior for unsupported dims (records skipped with error).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nudge the rows with careful paws,
Now 384 fits without a pause.
Columns line up, vectors sing,
Databases snug beneath my spring. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The pull request title "Fix/embedding 384" is vague and generic, using abbreviated terminology that lacks clear context. While it does refer to a real aspect of the change (fixing support for 384-dimensional embeddings), it is overly terse and does not convey the specific problem being addressed or what "384" refers to in the absence of additional context. A more descriptive title such as "Add support for 384-dimensional embedding models" would provide clearer information about the primary objective of the change. Consider revising the pull request title to be more descriptive and specific about what is being fixed. For example, "Add support for 384-dimensional embedding models in storage services" would better communicate the change to reviewers scanning the repository history. This would make it immediately clear that the PR addresses a missing feature or bug related to embedding dimension support without requiring readers to reference the issue number.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed The pull request implements the core requirements from issue #803. The changes add explicit support for 384-dimensional embeddings in both document_storage_service.py and code_storage_service.py by checking for embedding_dim == 384 and mapping it to the "embedding_384" column, preventing fallback to embedding_1536. This maintains consistency between the Python service logic and the database schema. The manual testing confirms that the fix resolves the original problem: files now upload successfully without warnings about unsupported embedding sizes. The code changes directly address the linked issue's primary objective of enabling 384-dimensional embedding model support.
Out of Scope Changes Check ✅ Passed All changes in this pull request are directly within scope and focused on the stated objective of fixing issue #803. The modifications are confined to adding embedding dimension handling for 384-dimensional vectors in document_storage_service.py and code_storage_service.py, with no unrelated refactoring, dependency updates, or extraneous file modifications. The changes maintain backward compatibility by preserving existing handling for other embedding dimensions (768, 1024, 1536, 3072) while adding the new 384 case.
Description Check ✅ Passed The pull request description follows the repository template structure and includes most required sections: a summary with issue reference, a "Changes Made" section, proper classification as a bug fix, testing notes, and a checklist. However, the "Affected Services" section has no services marked when the changes clearly affect the Server (FastAPI backend) component where document_storage_service.py and code_storage_service.py reside. Additionally, no new tests were added despite the scope of the changes affecting data handling logic, which limits verification of the fix. The description is mostly complete but lacks full accuracy in the service selection checkboxes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
python/src/server/services/storage/code_storage_service.py (1)

1310-1325: Consider adding tests for 384-dimensional embedding support.

Issue #803 specifically requested unit/integration tests covering embedding dimension selection (including the 384 case) and a regression test verifying successful batch insert for 384-d vectors. Adding these tests would prevent regressions and provide confidence that the fix works as intended.

Tests could cover:

  • Correct column selection for embedding_dim = 384
  • Successful batch insert of 384-d vectors into embedding_384 column
  • Error handling when embedding dimension is unsupported
  • Consistency between code_storage_service and document_storage_service

Based on learnings

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 37bc4a9 and 214ffdf.

📒 Files selected for processing (1)
  • python/src/server/services/storage/code_storage_service.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
python/src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

python/src/**/*.py: On service startup, missing configuration, DB connection failures, auth/authorization failures, critical dependency outages, or invalid/corrupting data: fail fast and bubble errors
For batch processing, background tasks, WebSocket events, optional features, and external API calls: continue processing but log errors (with retries/backoff for APIs)
Never accept or persist corrupted data; skip failed items entirely (e.g., zero embeddings, null FKs, malformed JSON)
Error messages must include operation context, IDs/URLs, use specific exception types, preserve full stack traces (logging with exc_info=True), and avoid returning None/null—raise exceptions instead; for batches report success counts and detailed failures
Backend code targets Python 3.12 and adheres to a 120 character line length
Use Ruff for linting (errors, warnings, unused imports) in backend code
Use Mypy for static type checking in backend code

python/src/**/*.py: Fail fast and loud on startup/config/auth/database/critical dependency failures and validation errors in backend services
Preserve full stack traces with logging.exc_info=True in Python logging
Use specific exception types and avoid catching generic Exception
Use Pydantic to raise on data validation errors; never silently accept bad data
Python 3.12 code should conform to a 120-character line length
Never return None to indicate failure in backend Python; raise exceptions with details instead

Files:

  • python/src/server/services/storage/code_storage_service.py
python/src/server/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Never accept corrupted data in backend flows; skip failed items entirely instead of storing bad data (e.g., zero embeddings, null FKs, malformed JSON)

Files:

  • python/src/server/services/storage/code_storage_service.py
python/src/server/services/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/src/server/services/**/*.py: For batch/background/optional operations, continue processing but log detailed per-item failures; report success counts and failure lists
Implement business logic in service modules, keeping database concerns separate
Place service logic under python/src/server/services/

Files:

  • python/src/server/services/storage/code_storage_service.py
🔇 Additional comments (2)
python/src/server/services/storage/code_storage_service.py (2)

1310-1311: LGTM! Embedding dimension handling now supports 384-d vectors.

The addition correctly maps 384-dimensional embeddings to the embedding_384 database column, preventing the dimension mismatch errors described in issue #803.


1310-1325: Embedding_384 schema verified and handled consistently
Confirmed the embedding_384 column exists in all migration scripts (including archon_code_examples) and is mapped identically in both storage services.

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.

🐛 [Bug]: support for embedding models of size 384 is broken

2 participants