chore: remove stale ONNX/tree-sitter/sqlitevec references from docs and build#16
Conversation
…nd build The ONNX embedding provider, ONNX reranker, tree-sitter chunkers (Python/TypeScript), and sqlite-vec have all been removed from the codebase. This commit updates all documentation, build files, and scripts to reflect the current state: - Embedding: OpenAI-compatible REST API only (no ONNX/builtin) - Reranking: API-based cross-encoder only (no ONNX) - Chunking: Markdown and Go (AST-based) only (no tree-sitter) - Vector store: PostgreSQL pgvector only (no sqlite-vec) Files updated: AGENTS.md, README.md, README.ru.md, README.zh.md, CONTRIBUTING.md, Dockerfile, Makefile, .goreleaser.yaml, .env.example, workflow-prepare.sh, docs/DEPLOYMENT.md, docs/arch/* (7 files), docs/src/App.vue Files deleted: scripts/download-onnx-libs.sh, scripts/download-bge-model.sh
ОбзорPR удаляет встроенную поддержку ONNX для встраивания и переводит систему на использование исключительно OpenAI-совместимого REST API для встраивания и переранжирования, удаляя связанные скрипты, конфигурацию сборки и обновляя документацию архитектуры. Изменения
Оценка трудозатрат на проверку кода🎯 4 (Сложный) | ⏱️ ~60 минут Возможно связанные PR
Рекомендуемые метки
Стихотворение
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly streamlines the project's architecture by removing several deprecated features and their associated build processes and documentation. The change consolidates embedding and reranking functionalities to rely solely on OpenAI-compatible REST APIs, and standardizes the vector store to PostgreSQL with pgvector. This cleanup reduces complexity, removes unnecessary dependencies, and clarifies the current technology stack, making the project easier to maintain and understand. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request does a great job of removing stale references to ONNX, tree-sitter, and sqlite-vec across documentation and build files. The changes are consistent and improve the clarity of the project's current architecture. I've found one area for improvement: the EMBEDDING_PROVIDER configuration variable is now obsolete as the provider is hardcoded to openai, but it hasn't been fully removed from all configuration examples and documentation. Removing it would prevent user confusion. I've added specific comments with suggestions to address this.
|
|
||
| # Embedding provider: "openai" (LiteLLM/OpenAI-compatible) or "builtin" (ONNX BGE, 384 dims) | ||
| # Embedding provider: "openai" (OpenAI-compatible REST API) | ||
| EMBEDDING_PROVIDER=openai |
| | `WORKER_HOST` | `0.0.0.0` | Worker bind address | | ||
| | `API_TOKEN` | — | Bearer token (recommended for remote) | | ||
| | `EMBEDDING_PROVIDER` | `onnx` | `onnx` (local BGE) or `openai` (REST API) | | ||
| | `EMBEDDING_PROVIDER` | `openai` | `openai` (OpenAI-compatible REST API) | |
| | `WORKER_HOST` | `0.0.0.0` | Адрес привязки worker | | ||
| | `API_TOKEN` | — | Bearer-токен (рекомендуется для удалённого доступа) | | ||
| | `EMBEDDING_PROVIDER` | `onnx` | `onnx` (локальный BGE) или `openai` (REST API) | | ||
| | `EMBEDDING_PROVIDER` | `openai` | `openai` (OpenAI-совместимый REST API) | |
| | `WORKER_HOST` | `0.0.0.0` | Worker 绑定地址 | | ||
| | `API_TOKEN` | — | Bearer 令牌(远程访问时推荐) | | ||
| | `EMBEDDING_PROVIDER` | `onnx` | `onnx`(本地 BGE)或 `openai`(REST API) | | ||
| | `EMBEDDING_PROVIDER` | `openai` | `openai`(OpenAI 兼容 REST API) | |
| | `ENGRAM_WORKER_PORT` | `37777` | Worker HTTP port (API + MCP) | | ||
| | `ENGRAM_API_TOKEN` | (empty) | Auth token for all endpoints | | ||
| | `ENGRAM_EMBEDDING_PROVIDER` | `builtin` | `builtin` or `openai` | | ||
| | `ENGRAM_EMBEDDING_PROVIDER` | `openai` | Embedding provider (`openai`) | |
| | `ENGRAM_EMBEDDING_MODEL` | string | `bge-v1.5` | ONNX model name for local (builtin) provider | | ||
| | `EMBEDDING_PROVIDER` | string | `builtin` | `builtin` or `onnx` = local ONNX; `openai` = REST API | | ||
| | `ENGRAM_EMBEDDING_MODEL` | string | `bge-v1.5` | Legacy field (unused — OpenAI provider only) | | ||
| | `EMBEDDING_PROVIDER` | string | `openai` | Embedding provider (`openai` = OpenAI-compatible REST API) | |
| Optional: OpenAI-compatible embeddings instead of local ONNX: | ||
| Configure embeddings (OpenAI-compatible REST API): | ||
| ```bash | ||
| export EMBEDDING_PROVIDER=openai |
|
@coderabbitai review |
|
@codex review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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 (3)
docs/DEPLOYMENT.md (1)
185-187:⚠️ Potential issue | 🟡 MinorРаздел про embeddings всё ещё обещает два провайдера.
Во вступлении секции написано, что провайдеров два, а ниже в этой же правке уже сказано, что ONNX/builtin удалён и остался только OpenAI-compatible REST API. Это выглядит как недочищенный stale reference и легко запутает читателя во время настройки.
Also applies to: 199-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/DEPLOYMENT.md` around lines 185 - 187, The intro line "Engram supports two embedding providers" is stale and conflicts with later text that states ONNX/builtin was removed; update the heading and any references to state a single provider (OpenAI-compatible REST API) instead of two, and remove or reword mentions of "ONNX/builtin" and "two providers" (look for the strings "Engram supports two embedding providers", "LiteLLM + Qwen3-Embedding-8B (recommended)" and "ONNX/builtin") so the section consistently describes only the remaining OpenAI-compatible REST API provider and the recommended LiteLLM + Qwen3-Embedding-8B setup..env.example (1)
14-20:⚠️ Potential issue | 🟠 MajorПример в
.env.exampleвсё ещё не совпадает с текущей схемой в документации.Здесь по умолчанию предлагается 4096-мерный embedding-конфиг, но
docs/arch/DATA_MODEL.mdиdocs/arch/GOTCHAS.mdв этом же PR по-прежнему описываютvector(384)и ручное пересоздание таблиц при смене размерности. Если пользователь просто скопирует этот блок в.env, он получит конфликтующую конфигурацию. Нужен один канонический сценарий: либо 384-dim пример, либо явное предупреждение о необходимости пересозданияvectorsиcontent_chunksпрямо здесь.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 14 - 20, The .env.example embedding defaults mismatch the docs: change EMBEDDING_DIMENSIONS from 4096 to 384 and update EMBEDDING_MODEL_NAME (and any example comment) to a 384-dim-compatible model so it matches docs/arch/DATA_MODEL.md and docs/arch/GOTCHAS.md; alternatively, if you prefer to keep 4096, add an explicit warning next to EMBEDDING_DIMENSIONS and EMBEDDING_MODEL_NAME stating that changing dimensions requires manual recreation of the vectors and content_chunks tables (and referencing EMBEDDING_DIMENSIONS, EMBEDDING_MODEL_NAME, EMBEDDING_PROVIDER).docs/src/App.vue (1)
429-429:⚠️ Potential issue | 🟠 MajorОсталась устаревшая формулировка про Python/TypeScript chunking.
На Line 429 всё ещё указано, что chunking делается для Go, Python и TypeScript. Это расходится с текущими изменениями (оставлены Markdown + Go) и ломает консистентность документации в этом же файле.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/App.vue` at line 429, Update the stale description string for the UI feature object whose title is "AST-aware chunking" in docs/src/App.vue: change the description that currently mentions "Go, Python, and TypeScript" to reflect the current supported languages ("Markdown + Go") so the description matches the recent changes and the rest of the documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/App.vue`:
- Around line 321-322: В разделе UI есть противоречие: один блок явно показывает
"OpenAI API" while другой по-прежнему говорит "No external services" / "local
embeddings is built-in"; приведите текст в соответствие: либо уберите упоминание
"OpenAI API" из заголовка/подписи, либо обновите требования (текст "No external
services"/"local embeddings is built-in") чтобы явно разрешать/уточнять
использование внешних сервисов и опциональность OpenAI API. Обновите
соответствующие строки с текстами "OpenAI API" и "No external services"/"local
embeddings is built-in" в App.vue так, чтобы сообщение пользователю было
однозначным.
In `@Makefile`:
- Around line 13-16: The Makefile currently exports CGO_ENABLED=1 and appends
BUILD_TAGS (fts5) to normal build targets so make build and the worker/mcp
targets depend on CGO/toolchain; change the Makefile so CGO_ENABLED and
BUILD_TAGS are only applied for test recipes: remove the global export of
CGO_ENABLED and stop passing $(BUILD_TAGS) to the worker and mcp build commands
used by the normal make build target, and instead set CGO_ENABLED=1 and
BUILD_TAGS="-tags fts5" only within the test target or test-specific rules
(e.g., in the recipe for the test or go test invocation) so make build, worker,
and mcp build without the test-only fts5 tag.
---
Outside diff comments:
In @.env.example:
- Around line 14-20: The .env.example embedding defaults mismatch the docs:
change EMBEDDING_DIMENSIONS from 4096 to 384 and update EMBEDDING_MODEL_NAME
(and any example comment) to a 384-dim-compatible model so it matches
docs/arch/DATA_MODEL.md and docs/arch/GOTCHAS.md; alternatively, if you prefer
to keep 4096, add an explicit warning next to EMBEDDING_DIMENSIONS and
EMBEDDING_MODEL_NAME stating that changing dimensions requires manual recreation
of the vectors and content_chunks tables (and referencing EMBEDDING_DIMENSIONS,
EMBEDDING_MODEL_NAME, EMBEDDING_PROVIDER).
In `@docs/DEPLOYMENT.md`:
- Around line 185-187: The intro line "Engram supports two embedding providers"
is stale and conflicts with later text that states ONNX/builtin was removed;
update the heading and any references to state a single provider
(OpenAI-compatible REST API) instead of two, and remove or reword mentions of
"ONNX/builtin" and "two providers" (look for the strings "Engram supports two
embedding providers", "LiteLLM + Qwen3-Embedding-8B (recommended)" and
"ONNX/builtin") so the section consistently describes only the remaining
OpenAI-compatible REST API provider and the recommended LiteLLM +
Qwen3-Embedding-8B setup.
In `@docs/src/App.vue`:
- Line 429: Update the stale description string for the UI feature object whose
title is "AST-aware chunking" in docs/src/App.vue: change the description that
currently mentions "Go, Python, and TypeScript" to reflect the current supported
languages ("Markdown + Go") so the description matches the recent changes and
the rest of the documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7b33f4c-74f5-44ff-95ff-5c4ddf1fe28c
📒 Files selected for processing (21)
.env.example.goreleaser.yamlAGENTS.mdCONTRIBUTING.mdDockerfileMakefileREADME.mdREADME.ru.mdREADME.zh.mddocs/DEPLOYMENT.mddocs/arch/COMPONENTS.mddocs/arch/CONFIGURATION.mddocs/arch/DATA_MODEL.mddocs/arch/GOTCHAS.mddocs/arch/OVERVIEW.mddocs/arch/QUICKSTART.mddocs/arch/architecture.mddocs/src/App.vuescripts/download-bge-model.shscripts/download-onnx-libs.shworkflow-prepare.sh
💤 Files with no reviewable changes (4)
- .goreleaser.yaml
- scripts/download-bge-model.sh
- scripts/download-onnx-libs.sh
- Dockerfile
| <div class="text-3xl sm:text-4xl font-bold text-amber-500 mb-2">OpenAI API</div> | ||
| <p class="text-slate-400 text-xs sm:text-sm">Two-stage retrieval: bi-encoder embeddings + cross-encoder reranking for high accuracy.</p> |
There was a problem hiding this comment.
Нужно устранить противоречие с разделом Requirements про внешние сервисы.
На Line 321 явно указан OpenAI API, но на Line 254–255 всё ещё сказано “No external services” и “local embeddings is built-in”. Это взаимоисключающие сообщения для пользователя.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/src/App.vue` around lines 321 - 322, В разделе UI есть противоречие:
один блок явно показывает "OpenAI API" while другой по-прежнему говорит "No
external services" / "local embeddings is built-in"; приведите текст в
соответствие: либо уберите упоминание "OpenAI API" из заголовка/подписи, либо
обновите требования (текст "No external services"/"local embeddings is
built-in") чтобы явно разрешать/уточнять использование внешних сервисов и
опциональность OpenAI API. Обновите соответствующие строки с текстами "OpenAI
API" и "No external services"/"local embeddings is built-in" в App.vue так,
чтобы сообщение пользователю было однозначным.
| # CGO settings (required for test build tags) | ||
| export CGO_ENABLED=1 | ||
| BUILD_TAGS := -tags "fts5" | ||
|
|
There was a problem hiding this comment.
make build всё ещё использует test-only CGO/fts5 настройки.
Комментарий теперь говорит, что CGO нужен только для test build tags, но CGO_ENABLED=1 экспортируется глобально, а worker и mcp по-прежнему собираются с $(BUILD_TAGS) при обычном make build. В итоге основной build-путь всё ещё зависит от CGO/toolchain, хотя документация в этом PR уже описывает это как test-only требование.
Возможный вариант, если `fts5` действительно нужен только тестам
-# CGO settings (required for test build tags)
-export CGO_ENABLED=1
-BUILD_TAGS := -tags "fts5"
+# Test-only settings
+TEST_BUILD_TAGS := -tags "fts5"
@@
- go build $(BUILD_TAGS) $(LDFLAGS) -o $(BUILD_DIR)/engram-server ./cmd/worker
+ CGO_ENABLED=0 go build $(LDFLAGS) -o $(BUILD_DIR)/engram-server ./cmd/worker
@@
- go build $(BUILD_TAGS) $(LDFLAGS) -o $(BUILD_DIR)/engram-mcp ./cmd/mcp
+ CGO_ENABLED=0 go build $(LDFLAGS) -o $(BUILD_DIR)/engram-mcp ./cmd/mcp
@@
- go test $(BUILD_TAGS) -v -race ./...
+ CGO_ENABLED=1 go test $(TEST_BUILD_TAGS) -v -race ./...
@@
- go test $(BUILD_TAGS) -v -race -coverprofile=coverage.out ./...
+ CGO_ENABLED=1 go test $(TEST_BUILD_TAGS) -v -race -coverprofile=coverage.out ./...As per coding guidelines, Makefile: Build the project using make build.
Also applies to: 26-26, 39-49, 129-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 13 - 16, The Makefile currently exports CGO_ENABLED=1
and appends BUILD_TAGS (fts5) to normal build targets so make build and the
worker/mcp targets depend on CGO/toolchain; change the Makefile so CGO_ENABLED
and BUILD_TAGS are only applied for test recipes: remove the global export of
CGO_ENABLED and stop passing $(BUILD_TAGS) to the worker and mcp build commands
used by the normal make build target, and instead set CGO_ENABLED=1 and
BUILD_TAGS="-tags fts5" only within the test target or test-specific rules
(e.g., in the recipe for the test or go test invocation) so make build, worker,
and mcp build without the test-only fts5 tag.
…ext inject
Two user-visible fixes bundled into one minimum-scope tactical PR:
1. MCP startup reliability after CC plugin update.
Smoking gun in .agent/reports/plugin-tactical-fix-triage.md: when CC
updates the plugin from v4.3.0 to v4.4.0, it does NOT migrate the
plugin-level user_config (server_url + api_token). The .mcp.json
then expands ${user_config.server_url} to an empty string, the
binary spawns silently, and every MCP tool call fails with an opaque
gRPC dial error against an empty target.
Fix: add a system-env fallback. plugin/engram/.mcp.json now also
passes ENGRAM_URL_LEGACY = ${ENGRAM_URL}. run-engram.js picks it up
if user_config-derived ENGRAM_URL is empty, and emits a visible WARN
to stderr if both are empty so the failure mode is no longer silent.
2. Disable noisy context injection.
Per .agent/reports/phase-2-synthesis.md fix #16 + the entity audits:
the session-start hook currently injects 100 raw observations
unioned with 10 semantic hits, plus project briefing, plus learned
guidance. The user reports the result as noise rather than relevant
context.
Fix: in plugin/engram/hooks/session-start.js, keep the inject GET
(still needed for result.always_inject) but render ONLY:
- result.always_inject -> <user-behavior-rules>
- GET /api/issues -> <engram-issues>
- GET /api/issues resolved -> <engram-resolved-issues>
Skip rendering of result.observations / result.full_count /
result.project_briefing / result.guidance. The mark-injected call
is now scoped to always_inject IDs only so citation tracking does
not log false positives for sections that were never shown.
Save / recall MCP tools (recall_memory, store_memory, find_by_file,
observations, store, feedback, vault, issues) are completely
independent of this hook — they go through a separate gRPC path and
are unaffected.
3. Plugin version bump 4.4.0 -> 4.4.1 per project rule (required for
marketplace cache invalidation).
Verification:
- JSON syntax check on .mcp.json and plugin.json — both parse clean
- node --check on run-engram.js and session-start.js — clean
- go build ./... clean (no Go-side changes, sanity check)
Files: 4 changed (the two scripts, the two JSON manifests).
Architectural reasoning + per-field classification recorded in
.agent/reports/plugin-tactical-fix-triage.md.
…REAKING) Consolidates the BREAKING CHANGE from PR #176 (legacy MCP/CLI removal) with two user-visible plugin tactical fixes into a single coherent v4.5.0 release. ## BREAKING CHANGES (carried over from PR #176) The 'engram-mcp' and 'engram-mcp-stdio-proxy' release artifacts no longer ship. Bare-metal users who installed via tarball or scripts/install.sh and relied on the engram-mcp binary must migrate to the plugin marketplace install path: /plugin marketplace add thebtf/engram-marketplace /plugin install engram Plugin marketplace users are unaffected — ensure-binary.js downloads only the 'engram' stdio-proxy binary, not engram-mcp. cmd/mcp/, cmd/engram-cli/, the Dockerfile client image stage, the engram-mcp Makefile + goreleaser + install.sh entries, and the bin/mcp-server / bin/mcp-stdio-proxy COMPONENTS.md sections are all gone. ## Fix #1 — MCP startup reliability after CC plugin update Smoking gun (.agent/reports/plugin-tactical-fix-triage.md): when CC updates the plugin from v4.3.0 to v4.4.x it does NOT migrate plugin-level user_config (server_url + api_token). .mcp.json then expands ${user_config.server_url} to an empty string, the binary spawns silently, and every MCP tool call fails with an opaque gRPC dial error against an empty target. Fix: - plugin/engram/.mcp.json: env block now also passes "ENGRAM_URL_LEGACY": "${ENGRAM_URL}" so users who had ENGRAM_URL in their shell from the v4.3.0 setup still get a working MCP server. - plugin/engram/scripts/run-engram.js: before spawning, fall back to ENGRAM_URL_LEGACY if ENGRAM_URL is empty, AND emit a visible WARN to stderr if both are empty so the failure mode is no longer silent. ## Fix #2 — disable noisy context injection Per .agent/reports/phase-2-synthesis.md fix #16 + the entity audits, the session-start hook currently injects 100 raw observations unioned with 10 semantic hits, plus project briefing, plus learned guidance. Reported as noise rather than relevant context. plugin/engram/hooks/session-start.js now keeps the inject GET (still needed for result.always_inject) but renders ONLY: - result.always_inject -> <user-behavior-rules> - GET /api/issues -> <engram-issues> - GET /api/issues resolved -> <engram-resolved-issues> Skipped: result.observations / result.full_count / result.project_briefing / result.guidance. mark-injected scoped to always_inject IDs only so citation tracking does not log false positives. Misleading observation- count log line removed. Save / recall MCP tools (recall_memory, store_memory, find_by_file, observations, store, feedback, vault, issues) are completely independent of this hook — separate gRPC path, unaffected. This is tactical, NOT a redesign. Phase 2 strategic work continues: citation session_id="" smoking gun, 100-obs cap policy, hook curated-render redesign — all in .agent/reports/phase-2-synthesis.md. ## Version bump Per Constitution §15 the daemon version and plugin version move together: - cmd/engram/main.go daemonVersion "v4.4.0" -> "v4.5.0" - plugin/engram/.claude-plugin/plugin.json "4.4.0" -> "4.5.0" ## Verification - JSON parse on .mcp.json + plugin.json: clean - node --check on run-engram.js + session-start.js: clean - go build ./... clean - 3-OS matrix CI green on the prior 4.4.1 commit (re-runs on amend) ## Files 4 plugin/daemon files + 1 daemon version constant. Net: -76 LOC (mostly removed render blocks in session-start.js).
Summary
Removes ~75 stale references to removed features across 21 files.
Removed references to:
Build cleanup:
download-onnx-libs.shfrom Dockerfile, Makefile, .goreleaser.yamldownload-onnx-libs.sh,download-bge-model.shworkflow-prepare.sh(removed ONNX + sqlite-vec patching)Docs updated:
Not modified: CHANGELOG.md (historical), migration names (factual records)
Test plan
go build ./cmd/worker/compilesgrep -ri "onnx" docs/ README* AGENTS.md CONTRIBUTING.mdreturns 0 hits (except CHANGELOG)Summary by CodeRabbit
Основные изменения
Значительные изменения
Документация