Skip to content

feat: add offline RAG knowledge base and harden retrieval - #7

Merged
Akaliup merged 5 commits into
masterfrom
feat/agent-evaluation-trace
Nov 1, 2026
Merged

Akaliup merged 5 commits into
masterfrom
feat/agent-evaluation-trace

Conversation

@Akaliup

@Akaliup Akaliup commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

摘要

  • 增加离线优先的运营知识库:knowledge_documents / knowledge_chunks、Markdown 字符分块、hashing / OpenAI-compatible embedding、混合检索、no_answer、引用校验、知识库 API 与前端面板。
  • 增加 RAG 离线评测、HTTP 冒烟与前端测试覆盖。
  • 修复独立审查发现的 P1:
    • 合并预检索与工具检索命中后,按分数截断到 10 条并重新做 Schema 校验,避免下游 KnowledgeSearchOutput 验证失败。
    • retrieve_knowledge 不再受通用 step_timeout_seconds 事后检查,远程 embedding 超时降级为 knowledge_error,不阻断确定性分析。
    • 知识内容和工具结果统一清洗、常见凭据脱敏,并强化提示词隔离。
  • 修复第二轮审查的四个 P2(提交 044bd1e):
    1. 引用校验改为使用本次实际提供给模型的 chunk_id 集合(预检索 payload + 工具结果中可见 ID);合并面板最多 10 条时优先保留被 LLM 引用的 chunk,低分有效引用不再被误判。
    2. 远程 embedding 不可用时启动降级:知识库 Seed 事务回滚,保留已有文档,/health 与 /api/demo 返回 knowledge.seed_error;数据库初始化失败仍中止启动。
    3. 修复 _split_long 在“短句 + 超长句 + 短句”场景的顺序问题,保留原文顺序、chunk 长度上限与 overlap。
    4. OpenAI-compatible embedding 响应严格校验 null、非数字、NaN、Infinity、空向量、重复/缺失 index 和维度不一致,统一映射 knowledge_embedding_failed(502),非法向量不落库。
  • 修复第三轮 Codex Review 的两个 P2(最新提交 ab79d12):
    1. embedding 模型切换后不再因相同正文直接返回 created=false 而跳过索引:在原文档上重嵌入现有 chunk,保留文档 ID、source、version、effective_date、active 与 chunk ID/正文;新增 POST /api/knowledge/documents/{id}/reindex(仅 approver),重复执行幂等,失败事务回滚,不静默降级为 hashing。
    2. Markdown 标题写入 knowledge_chunks.heading 前截断到 200 字符,覆盖 200/201/500 字符边界;正文与段落顺序不变,避免 PostgreSQL String(200) 写入失败。
  • 本 PR 基于已合并的 Phase 2D 分支;P1 修复为 cff917d,第二轮 P2 为 044bd1e,第三轮 P2 为 ab79d12。

实际执行的验证

环境:Windows 11、Python 3.12.8、Node 24.16.0。

命令 结果
git status 本轮修改前 clean at 044bd1e;提交并推送后远端 feat/agent-evaluation-trace = ab79d12
.venv/Scripts/python.exe -m pytest backend/tests -q 174 passed,1 skipped(PostgreSQL 环境变量未设置;1 个第三方 DeprecationWarning)
PROFITOPS_TEST_POSTGRES_URL=... pytest backend/tests/test_knowledge.py::test_postgres_accepts_bounded_long_heading 1 passed(一次性 PostgreSQL 16 容器,500 字符标题实际写入成功)
npm --prefix frontend test 15 passed
npm --prefix frontend run build 通过(Vite 6.4.5,2252 modules)
.venv/Scripts/python.exe scripts/smoke.py --base-url http://127.0.0.1:18013 15/15 passed(独立 SQLite,LLM_API_KEY="",端口 18013)
npm --prefix frontend run test:e2e 3/3 passed(真实 Chromium,独立 data/e2e.db)
.venv/Scripts/python.exe backend/evals/rag.py 6 篇文档、8 个查询;hit@1 83.33%、hit@3 100%、no-answer 100%、citation validity 100%、网络调用 0
GitHub CI 2/2 SUCCESS(Backend pytest、Frontend tests and production build,ab79d12);mergeStateStatus: CLEAN

新增回归覆盖:

  • 超过 10 条不同 Chunk、低分 Chunk 被引用时仍通过校验并保留在持久化面板。
  • httpx.MockTransport 模拟远程 embedding 超时,核心 API 启动成功且 knowledge.seed_error 可见;已有文档保留;数据库初始化失败仍抛出。
  • “短句 + 超长句 + 短句”分块顺序、长度与 overlap。
  • null、非数字、NaN、Infinity、空向量、重复 index、维度不一致等异常 embedding 响应统一 502,且不写入数据库。
  • hashing-v1 → 确定性 FakeEmbedding 新模型:重复上传相同正文触发 reindex,文档与 chunk ID/正文保留,旧模型检索不再命中,新模型检索恢复;重复 reindex 不再调用 embedding。
  • reindex 失败回滚:旧向量与 embedding_model 保持不变,旧模型仍可检索;analyst 调用 reindex 返回 403,文档数量不增加。
  • 200/201/500 字符 Markdown 标题:存储 heading 不超过 200,正文完整且段落顺序不变;PostgreSQL 16 实际写入验证通过。

未验证

  • 真实 LLM 调用:未执行,未消耗付费 Token。
  • 真实 embedding 服务(Ollama / vLLM / 云端):未连接;OpenAI-compatible Provider 仅用 httpx.MockTransport 测试。
  • PostgreSQL:仅用一次性 PostgreSQL 16 容器验证了 500 字符标题的写入边界;完整 PostgreSQL 业务回归与 Docker Compose 全栈未启动。
  • PDF / OCR / TXT 文件入库:未实现。
  • 大规模向量索引 / ANN / 延迟:未测试;当前是 Python 全量扫描 cosine。
  • 知识库专项 Playwright 断言:现有 E2E 3/3 通过,但未新增知识库页面专项断言。
  • 真实 LLM / 真实 embedding 服务的行为仍只能由 MockTransport 覆盖;CI 通过不代表真实远程服务可用。

已知限制与后续 P2

  • 默认 hashing embedding 是词法特征哈希,不是语义向量;8 条合成查询的 hit@3=100% 不是真实语义检索准确率。
  • version / effective_date 不参与检索;没有文档级 ACL、租户隔离、reranker 或向量数据库。
  • 工具结果被字节上限截断时,只有 preview 中仍可见的 chunk_id 可被引用;被截断的片段引用按 fail-closed 处理。
  • 文档时间戳 Z、no_answer 弃答、创建竞态、失败状态 UI、Promise.all、citation 评测自证等 P2 未在本 PR 处理。
  • reindex 只重嵌入现有 chunk,不重新切分;修改 chunk 参数后仍需重新入库。超过 200 字符的完整标题不会单独保存。
  • 知识注入已做提示词、工具结果清洗和脱敏,但不是形式化防注入保证;真实模型行为未验证。

审查重点

  • 整数美分与利润守恒、证据 ID 可追溯、审批与执行幂等、服务端角色校验、无真实外部副作用。
  • 不自动合并;合并前需要 CI 通过并完成人工审查。

🤖 Generated with Claude Code

lz and others added 3 commits November 1, 2026 15:27
- add knowledge documents/chunks, Markdown chunking, deterministic hashing embeddings and optional OpenAI-compatible embeddings
- add hybrid retrieval with no-answer threshold, retrieve_knowledge workflow node and search_knowledge tool
- validate LLM knowledge references and expose cited chunks in analyses and recommendations
- add knowledge APIs, frontend knowledge workspace and citation panels
- add offline RAG evaluation, knowledge tests and documentation

Co-Authored-By: Claude Code <noreply@anthropic.com>
- Cap merged knowledge hits to the schema limit, re-rank, and revalidate before downstream use.\n- Exclude knowledge retrieval from the generic step timeout so slow embeddings degrade to knowledge_error.\n- Sanitize and redact knowledge tool results, and strengthen prompts against embedded instructions and credential leakage.

Co-Authored-By: Claude Code <noreply@anthropic.com>
- Validate citations against chunk IDs actually provided to the model and prefer cited chunks when capping merged hits.\n- Degrade startup on remote embedding failure, expose seed_error, and keep database failures fatal.\n- Preserve short/long sentence order during chunking.\n- Reject malformed embedding values before persistence.\n\nCo-Authored-By: Claude Code <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T10:06:25.309607Z 7fd0625 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Co-Authored-By: Claude Code <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cff917dcbf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/app/knowledge.py
Comment on lines +552 to +555
merged.sort(key=lambda hit: hit.score, reverse=True)
merged = [
hit.model_copy(update={"rank": rank})
for rank, hit in enumerate(merged[:KNOWLEDGE_MAX_HITS], start=1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve valid citations when capping merged retrieval results

When pre-retrieval and tool searches return more than ten unique chunks, this truncation can discard chunks already supplied to the model. verify_llm subsequently validates citations only against the truncated result, so citing one of those previously supplied chunks incorrectly triggers invalid_knowledge_reference, discards an otherwise valid LLM response, and can fail the analysis under the fail policy. Keep the complete set of supplied citation IDs for validation, and preserve cited chunks in the persisted context rather than selecting solely by retrieval score.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment thread backend/app/main.py Outdated
initialize_database(engine)
if settings.demo_mode and settings.auto_seed:
seed_demo(factory)
seed_knowledge(factory, embeddings, settings)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow startup to continue when remote knowledge seeding fails

On a fresh database with KNOWLEDGE_EMBEDDING_PROVIDER=openai and the default enabled demo/auto-seed settings, this startup call embeds all seed documents through the remote service. A timeout or HTTP error propagates out of the lifespan before yield, preventing the entire API from starting, including deterministic analysis and health endpoints. The retrieval node's error-degradation handling cannot help because startup never completes. Handle optional knowledge-seeding failures separately so a temporarily unavailable embedding service does not take down the application.

Useful? React with 👍 / 👎.

Comment thread backend/app/knowledge.py
Comment on lines +287 to +289
if len(sentence) > max_chars:
for index in range(0, len(sentence), max_chars):
units.append(sentence[index:index + max_chars])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Flush buffered sentences before splitting a long sentence

If a paragraph contains a short sentence followed by a sentence longer than max_chars, the short sentence remains in current while the later sentence is appended directly to units; the buffered text is emitted afterward. For example, _split_long('First. ' + 'B' * 25 + '.', 10) returns the B fragments before First.. This corrupts the original ordering of indexed SOP text and can attach introductory conditions to the wrong subsequent instructions. Flush and clear current before appending fragments of the oversized sentence.

Useful? React with 👍 / 👎.

Comment thread backend/app/knowledge.py Outdated
Comment on lines +186 to +190
return [self._normalize(vector) for vector in vectors]

@staticmethod
def _normalize(vector: list[Any]) -> list[float]:
values = [float(value) for value in vector]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Map invalid embedding values to the documented provider error

If an embedding endpoint returns a nonnumeric or null vector element, such as {"data":[{"index":0,"embedding":[null]}]}, the response passes the list checks but raises TypeError or ValueError during normalization outside the response-validation handler. Direct knowledge-search and document-creation requests therefore return an unhandled 500 instead of the documented knowledge_embedding_failed 502. Include numeric conversion in the guarded validation and reject non-finite values before storing or scoring vectors.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab79d125b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/app/knowledge.py
Comment on lines +529 to +531
stale = [
chunk for chunk in chunks
if chunk.embedding_model != embeddings.model or not chunk.embedding

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Detect dimension changes when deciding whether to reindex

If an existing hashing knowledge base is restarted with a different KNOWLEDGE_EMBEDDING_DIMENSIONS value, the model name remains hashing-v1, so this check treats the old vectors as current. Both explicit reindexing and re-uploading identical content therefore become no-ops, while _cosine returns zero for every old chunk because its dimensions differ from the query vector. Retrieval silently loses its vector component and can reject previously answerable queries. Include the vector dimensions in the embedding identity or freshness check so reindexing can repair this supported configuration change.

Useful? React with 👍 / 👎.

Comment thread backend/app/knowledge.py
Comment on lines +471 to +475
existing = session.scalar(
select(KnowledgeDocument).where(
KnowledgeDocument.content_sha256 == sha,
KnowledgeDocument.active.is_(True),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce document deduplication atomically

In the PostgreSQL deployment, two concurrent uploads of the same new content can both observe no existing document here and then both insert successfully: KnowledgeDocument.content_sha256 has only a non-unique index. This violates the documented creation idempotency and produces identical chunks with different IDs, allowing duplicate results to consume retrieval slots. Enforce uniqueness for active content hashes, or otherwise serialize creation, and return the existing document when another request wins the race.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 044bd1e437

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/app/knowledge.py Outdated
Comment on lines +465 to +469
if existing is not None:
count = int(session.scalar(
select(func.count()).select_from(KnowledgeChunk).where(KnowledgeChunk.document_id == existing.id)
) or 0)
return knowledge_document_output(existing, count, created=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reindex duplicate content when the embedding model changes

After changing KNOWLEDGE_EMBEDDING_MODEL, uploading an existing document again returns created=false without generating embeddings for the new model because deduplication checks only the content hash and active flag. Meanwhile, search_knowledge excludes all chunks whose embedding model differs, so the documented re-ingestion path leaves these documents permanently absent from search unless users first deactivate and recreate them. Make the duplicate-content path account for the current embedding model or provide an explicit reindex operation.

Useful? React with 👍 / 👎.

Comment thread backend/app/knowledge.py
text = str(chunk["content"])
session.add(KnowledgeChunk(
id=new_id(), document_id=document.id, chunk_index=index,
heading=chunk["heading"], content=text, char_count=len(text),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bound Markdown headings before persisting chunks

In the PostgreSQL Compose deployment, an otherwise valid document containing a Markdown heading longer than 200 characters causes document creation to fail with an unhandled database error: chunk_text retains the entire heading, but KnowledgeChunk.heading is declared as String(200). The request schema limits total content length, not individual headings, so this input passes API validation and only fails when the transaction flushes. Validate heading length or truncate the stored heading before insertion so supported document uploads do not produce HTTP 500 responses.

Useful? React with 👍 / 👎.

- Reindex when a same-model provider returns a different vector size, not
  only when the model name changes.
- Enforce one active document per content SHA-256 with a partial unique index;
  migrate legacy duplicates by soft-deleting all but the earliest row.
- Serialize SQLite uploads with BEGIN IMMEDIATE and recover PostgreSQL insert
  conflicts in a savepoint, reusing the winning document.
- Skip stale-dimension chunks during retrieval and add dimension, migration,
  and two-request concurrency tests for SQLite and PostgreSQL.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@Akaliup
Akaliup merged commit 0ae784b into master Nov 1, 2026
2 checks passed
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