-
Notifications
You must be signed in to change notification settings - Fork 0
Extract vectoria db from repo #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThe PR removes the vectoriadb library and its docs/demo/configs, adds/updates multiple GitHub Actions workflows, and integrates optional VectoriaDB similarity scoring into enclave-vm's LocalLlmScorer with heuristic fallback and expanded tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Scorer as LocalLlmScorer
participant VDB as VectoriaDB
participant Heu as HeuristicAnalyzer
participant CA as CustomAnalyzer
Client->>Scorer: scoreWithSimilarity(features, toolCalls)
Scorer->>Scorer: check mode == "similarity"
Scorer->>Scorer: initializeVectoriaDB() (dynamic import)
alt VectoriaDB available
Scorer->>VDB: initialize()/load(indexPath?, modelName)
VDB-->>Scorer: initialized
Scorer->>VDB: search(query, {topK, threshold})
VDB-->>Scorer: searchResults
Scorer->>Scorer: derive similarityScore + emit SIMILARITY_MATCH
else VectoriaDB unavailable
Scorer->>Heu: analyze(features)
Heu-->>Scorer: heuristicScore
end
Scorer->>CA: (optional) analyzeWithCustomModel(features, prompts)
CA-->>Scorer: customScore
Scorer->>Scorer: merge signals -> finalScore
Scorer-->>Client: finalScore + signals
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/enclave-vm/src/scoring/types.ts (1)
343-375: Document VectoriaConfigForScoring in the public API reference.The new
VectoriaConfigForScoringinterface with itstopKandmodelNameoptions is not documented indocs/api-reference/enclave-vm.mdx. As a public SDK API, this requires documentation updates per coding guidelines. Add the interface definition and field descriptions to the scoring configuration section of the API reference.
🤖 Fix all issues with AI agents
In @.github/workflows/create-release-branch.yml:
- Around line 307-320: The heredoc written to "${{ env.CODEX_OUTPUT }}" is
indented, so the created JSON will include leading whitespace and become
invalid; fix by making the heredoc body left-aligned (remove all leading
indentation for the JSON block that uses PROJECTS_JSON and BUMP_TYPE) or switch
to a tab-stripped heredoc (<<-EOF) and ensure the JSON lines are indented with
tabs only; update the block that writes to "${{ env.CODEX_OUTPUT }}" so the
opening marker and the JSON payload are at column 0 (or use <<-EOF with tabs) so
consuming node scripts can parse the JSON.
In `@libs/enclave-vm/src/scoring/scorers/local-llm.scorer.ts`:
- Around line 22-33: The VectoriaDBInstance interface incorrectly includes an
optional loadIndex(path) method; remove that method from the interface and
ensure the type only exposes initialize(): Promise<void> and search(query:
string, options?: { topK?: number; threshold?: number }):
Promise<VectoriaSearchResult[]> (leave VectoriaSearchResult unchanged), or if
persistence is required update usage to call VectoriaDB's
saveToStorage()/storage adapter APIs instead of loadIndex; update any code
referencing VectoriaDBInstance.loadIndex to use the correct persistence
mechanism.
🧹 Nitpick comments (2)
.github/workflows/create-release-branch.yml (1)
274-284: Missing default case for unexpected bump_type values.The case statement handles
major,minor, andpatchbut doesn't have a fallback*case. While the workflow_dispatch input restricts values, a defensive default would prevent silent failures if the input validation changes.Suggested fix
case "$BUMP_TYPE" in major) NEW_VERSION="$((MAJOR + 1)).0.0" ;; minor) NEW_VERSION="${MAJOR}.$((MINOR + 1)).0" ;; patch) NEW_VERSION="${MAJOR}.${MINOR}.$((PATCH + 1))" ;; + *) + echo "::error::Unexpected bump type: $BUMP_TYPE" + exit 1 + ;; esac.github/workflows/trigger-docs-update.yml (1)
174-184: Consider adding version_minor to the summary.The
version_minoris computed and included in the dispatch payload but not displayed in the step summary. Adding it would improve visibility for debugging.Suggested addition
echo "- **SHA:** ${{ github.sha }}" >> $GITHUB_STEP_SUMMARY echo "- **Diff base:** ${{ steps.diff_base.outputs.diff_base }}" >> $GITHUB_STEP_SUMMARY + echo "- **Version:** ${{ steps.diff_base.outputs.version_minor }}" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/create-release-branch.yml (1)
208-220: Heredoc indentation will create malformed TOML.Similar to the JSON heredoc issue, the leading whitespace from YAML indentation will be included in the TOML output. Lines like
[features]with leading spaces are not valid TOML section headers.Proposed fix - remove leading indentation
cat > "$CODEX_HOME/config.toml" << 'EOF' - [features] - # Enable the Rust MCP client (needed for HTTP/OAuth MCP support) - rmcp_client = true - - # --- Mintlify Documentation Server --- - # Provides access to Mintlify documentation best practices - [mcp_servers.mintlify_docs] - url = "https://mintlify.com/docs/mcp" - http_headers = { "X-MCP-Readonly" = "true" } - startup_timeout_sec = 30 - tool_timeout_sec = 60 - EOF +[features] +# Enable the Rust MCP client (needed for HTTP/OAuth MCP support) +rmcp_client = true + +# --- Mintlify Documentation Server --- +# Provides access to Mintlify documentation best practices +[mcp_servers.mintlify_docs] +url = "https://mintlify.com/docs/mcp" +http_headers = { "X-MCP-Readonly" = "true" } +startup_timeout_sec = 30 +tool_timeout_sec = 60 +EOF
♻️ Duplicate comments (1)
.github/workflows/create-release-branch.yml (1)
366-379: Heredoc indentation will create malformed JSON.This issue was already flagged in a previous review. The heredoc content is indented to match the YAML structure, but this indentation will be included in the output file, creating invalid JSON that the
nodescripts cannot parse.
🧹 Nitpick comments (2)
.github/workflows/create-release-branch.yml (2)
349-354: Simplify misleading conditional.The
if/elseblock checksIS_COREbut both branches assignLIB_VERSION="$NEW_VERSION". The inline comment explains this is intentional for simplicity, but the conditional is unnecessary and could confuse readers.Proposed simplification
- # Core libs use unified version, others use their own - if [ "$IS_CORE" = true ]; then - LIB_VERSION="$NEW_VERSION" - else - LIB_VERSION="$NEW_VERSION" # For simplicity, use same version for all in manual mode - fi + # For manual mode, all libs use the same unified version + LIB_VERSION="$NEW_VERSION"
411-414: Consider updating error message for accuracy.The error message "Codex output file not found" is misleading when running in manual mode, since the file is generated by the manual bump step, not Codex.
Proposed fix
if [ ! -f "${{ env.CODEX_OUTPUT }}" ]; then - echo "::error::Codex output file not found" + echo "::error::Release output file not found at ${{ env.CODEX_OUTPUT }}" exit 1 fi
… from VectoriaDB integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@libs/enclave-vm/src/scoring/scorers/local-llm.scorer.ts`:
- Around line 305-348: The early-return that delegates to this.fallbackScorer
when this.pipeline is falsy is incorrectly unconditional and prevents
similarity-mode logic from running; update the fallback check (where it
currently returns early to call this.fallbackScorer) to only fallback when mode
!== 'similarity' (or when this.config.mode isn't 'similarity'), so that when
this.config.mode === 'similarity' the code proceeds to call scoreWithSimilarity
(and uses VectoriaDB/heuristics) even if this.pipeline is falsy; locate the
conditional that references this.pipeline and this.fallbackScorer and add the
mode check to skip the fallback in similarity mode.
In `@libs/enclave-vm/src/scoring/types.ts`:
- Around line 343-369: Document the breaking API change to
VectoriaConfigForScoring (libs/enclave-vm/src/scoring/types.ts): add a new docs
page or section under the ai-scoring documentation describing the removal of
indexPath (explain why it was removed and give a clear migration path for users
who relied on indexPath), document the added fields topK and modelName including
defaults and usage examples, and include a short compatibility note and
changelog entry indicating this is a breaking change and how to adapt existing
configs for VectoriaDB/VectoriaConfigForScoring.
… from VectoriaDB integration
Summary by CodeRabbit
Removals
New Features
Updates
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.