fix(semantic): gate non-substantive content out of L0/L1 generation - #3049
fix(semantic): gate non-substantive content out of L0/L1 generation#3049chethanuk wants to merge 8 commits into
Conversation
|
Please review cc: @qin-ctx @ZaynJarvis @zhoujh01 @yufeng201 @chenjw - Please let me know if you require any changes, or if the project is currently accepting contributions |
|
@chethanuk @qin-ctx 我在独立 worktree 将 #3049 head
建议作者按以上三点更新现有分支；更新后这份 current-main 审查没有发现需要改写整体方案的结构性阻塞。 |
16745af to
1009e39
Compare
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
Re-review of exact head 1009e39 against current main 09cd478: the three prior blockers are fixed. parser_config preserves SemanticConfig.post_init while adding min_substantive_chars; both substantive test paths now isolate render_prompt; the focused suite passes 39/39 in a no-credential HOME; Ruff 0.15.5 check and format --check pass; and the head directly contains current main.
One correctness gap remains before approval: when a previously substantive file or directory becomes non-substantive, semantic_dag.py only suppresses scheduling a new file task at line 684 and a new directory task around lines 829-841. The memory path likewise filters/skips new vectorization around semantic_processor.py lines 798-863. None of those transitions removes the pre-existing DETAIL or L0/L1 vector records, so the old substantive or hallucinated vectors remain searchable after the source is rewritten to heading-only content. That still violates #3028 expected behavior that non-substantive files not be vectorized as meaningful content and leaves the retrieval pollution in place for updates.
Please add transition cleanup for both the file DETAIL vector and directory L0/L1 vectors, plus a regression test that starts with already-vectorized substantive content, rewrites it to heading-only/non-substantive content, and proves the old records are removed rather than merely not refreshed.
Regex-only has_substantive_content() strips markdown structure (frontmatter, comments, headings, setext/HR, list/table/quote markers, code fences, link and image markup, emphasis) keeping code body, cell text, and anchor/alt text, then measures a CJK-weighted residual length. Adds SemanticConfig. min_substantive_chars (default 8, weighted). Pure function + config knob only; pipeline wiring follows. Table-driven test covers all issue edge cases incl. CRLF and bold-run bodies. Refs volcengine#3028
Empty/title-only/heading-only documents were sent to the VLM for summary generation, producing hallucinated abstracts/overviews that polluted vector search (issue volcengine#3028). Wire has_substantive_content through the semantic pipeline: - _generate_text_summary short-circuits before the VLM for non-substantive doc/generic text (untruncated content; code and media stay substantive), returning has_substantive_content=False. - Flag propagates through file_summaries; _finalize_file_summaries None-fallback defaults it False. - _file_summary_task skips file vectorization when the flag is False. - _generate_overview filters non-substantive summaries; when nothing substantive remains it writes a deterministic neutral overview with no VLM call, marked '[Directory has no substantive content]'. - Both directory write paths skip vectorization for the neutral overview; the reindex guard (_is_not_ready_sentinel) recognizes the new marker so it is never embedded as an L0/L1 vector (consistent with volcengine#2434). Refs volcengine#3028
The per-file summary dict now holds a bool flag alongside string fields, so DirNode.file_summaries, VectorizeTask.summary_dict and _finalize_file_summaries are typed Dict[str, Any] instead of Dict[str, str]. Keeps mypy clean (no new errors from volcengine#3028). Refs volcengine#3028
- has_substantive_content: strip EOF-terminated frontmatter (no trailing newline), so a frontmatter-only file at end-of-file is non-substantive. - _process_memory_directory: gate per-file vectorization on has_substantive_content, matching the DAG path — non-substantive files in the memory pipeline are no longer embedded. - _generate_overview: when the VLM is unavailable, return the transient not-ready marker (not the permanent no-content marker), so a directory with real content is not mislabeled empty while the model is merely down. Refs volcengine#3028
Review follow-up: the function returned a (bool, int) tuple whose int half was never consumed (the telemetry claim was unrealized), and carried a cjk_weight parameter that was never overridden. Return a plain bool and inline the weight as the module constant _CJK_WEIGHT. No behavior change. Refs volcengine#3028
Drives the real read->detect->gate->overview path (_generate_single_file_summary -> _generate_text_summary -> read_file, _generate_overview) against real .md files written to disk, with a spy VLM. Proves the VLM is bypassed for heading-only/frontmatter-only content and awaited for substantive content, and that an all-non-substantive directory yields the neutral overview the reindex guard treats as non-embeddable. Stubs only external services (VLM, config, file backend). Refs volcengine#3028
The substantive-content gate tests monkeypatch semantic_processor's get_openviking_config, but the substantive-file path still calls render_prompt, which lazily builds a global PromptManager that reads the real machine config — making the tests environment-dependent and able to serialize local config into a failure traceback. Stub semantic_processor.render_prompt next to each config patch so the tests are hermetic (mirrors test_semantic_processor_overview_batching). Also apply ruff format to semantic_processor.py after the rebase onto current main.
Suppressing new vectorization was not enough: records are upserts keyed on uri+level, so a file rewritten from substantive to heading-only content kept its old DETAIL record searchable, and a directory whose overview turned neutral kept stale L0/L1 records. Add idempotent exact-URI cleanup (viking_fs._delete_from_vector_store, the same helper rm/mv use) at the three suppression points: the DAG file task (content changed and now non-substantive), the DAG overview task (inside the neutral-overview branch only — the write-failure suppression keeps vectors), and the memory path (stale changed files + neutral directory). A file's exact URI holds only its DETAIL record and a directory's only its L0/L1, so no level plumbing is needed. Summaries reconstructed from cached overviews lack the substantive flag and default to kept. Regressions: rewrite-to-non-substantive deletes the file/dir records (seeded first, proven removed); write-failure suppression deletes nothing; memory-path variants for both hunks.
1009e39 to
0b22691
Compare
|
Addressed the transition gap: a substantive→non-substantive rewrite now deletes the stale DETAIL / dir L0/L1 records at all three suppression points instead of just skipping the refresh. Added regressions that vectorize first, rewrite to heading-only, and prove the records are actually removed (plus one showing the write-failure path keeps existing vectors). Rebased on current main (49ca3cd), 44/44 focused tests green. |
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 0b22691.
[P1] Preserve directory L0/L1 when the neutral sidecar write is suppressed or fails. In _overview_task, the neutral-overview branch calls _delete_from_vector_store([dir_uri]) before _write_directory_semantics. If write_semantic_sidecars returns False (for example for a stale/coalesced message or a suppressed write), the existing directory vectors have already been deleted even though no replacement sidecar was committed. The current write-failure regression uses substantive content and does not enter this neutral branch. A focused reproduction with a heading-only child and write_semantic_sidecars = AsyncMock(return_value=False) records deletion of both the file URI and the directory URI before the failed write. Please defer the directory-vector deletion until the sidecar write succeeds, and add a neutral-overview write-failure/stale regression.
Validation on this exact head: 44 focused tests passed; Ruff check and format-check passed for all 8 changed Python files; merge-tree against current origin/main is conflict-free; required CI checks are green.
2 participants
Description
Empty, whitespace-only, or heading/title-only Markdown documents were being sent to the VLM for L0/L1 (abstract/overview) generation. With no substantive content the model invents a plausible-but-nonexistent purpose, audience, and keywords, polluting
.abstract.md,.overview.md, and vector search. For example, a file containing only# Example Wiki Pageproduced a fabricated overview.This change adds a substantive-content gate to the semantic pipeline. Non-substantive text is detected before any model call, so the VLM never sees it — it cannot hallucinate a summary for content that isn't there — and such content is neither vectorized nor aggregated into directory overviews. A directory with no substantive input gets a deterministic, neutral overview instead of an invented one.
flowchart TD A[Text file content] --> B{has_substantive_content?<br/>markup stripped, CJK-weighted} B -- No --> C[Empty summary<br/>has_substantive_content = False] C --> D[Skip VLM call] C --> E[Skip file vectorization] B -- Yes --> F[VLM summary as before] F --> G[Vectorize file] E --> H{Any substantive summary<br/>or child abstract in dir?} G --> H H -- No --> I["Neutral overview (no VLM)<br/>'[Directory has no substantive content]'"] I --> J[Skip directory embedding<br/>reindex guard recognizes marker] H -- Yes --> K[Generate directory overview as before]Related Issue
Fixes #3028
Type of Change
Changes Made
has_substantive_content(text, min_chars=8)insemantic_processor.py— regex-only Markdown strip (frontmatter, HTML comments, headings, setext/HR, list/table/blockquote markers, code-fence markers, link/image markup, emphasis) that keeps code bodies, table cell text, and link/image label text, then measures a CJK-weighted residual length. Normalizes CRLF first (issue reported on Windows).SemanticConfig.min_substantive_chars(default8, weighted) so the threshold is tunable without a code change._generate_text_summary: non-substantive documentation/generic text short-circuits before the VLM and returnshas_substantive_content=Falseon untruncated content. The code branch and media helpers stay substantive.file_summaries/summary_dict;_finalize_file_summariesdefaults the flag toFalseon itsNone-fallback._file_summary_task._generate_overviewfilters non-substantive summaries; when nothing substantive remains it writes a deterministic neutral overview marked[Directory has no substantive content]with no VLM call, in both the DAG and memory write paths, and skips directory embedding.reindex_executor._is_not_ready_sentinelnow also recognizes the new no-content marker, so a neutral overview/abstract is never embedded as an L0/L1 vector (consistent with the existing not-ready placeholder handling).Testing
tests/storage/test_has_substantive_content.py— 20 table-driven cases covering every edge case in the issue (empty, whitespace, ATX/setext heading-only, heading+body, frontmatter-only, frontmatter-at-EOF, HTML-comment-only, table, links-with-text, bare-URL, image alt / no-alt, code-only, CJK title-only, CJK heading+body, divider-only) plus a CRLF case (# Title\r\n→ non-substantive) and a bold-run body, so the Windows-reported path is exercised on Linux.tests/storage/test_semantic_substantive_gate.py— pipeline behavior: non-substantive file skips the VLM and is not vectorized; a substantive file still calls the VLM and is flagged; overviews filter out non-substantive summaries; an all-non-substantive directory writes the neutral overview/abstract with no VLM call and is not vectorized; a transient VLM outage keeps the retryable not ready marker (not the permanent no content one); and the reindex guard treats the neutral overview as non-embeddable.tests/storage/test_semantic_gate_e2e.py— end-to-end on real files: writes real.mdfiles to disk and drives the actual read → detect → gate → overview path (_generate_single_file_summary→_generate_text_summary→read_file, then_generate_overview) with a spy VLM, asserting the model is bypassed for heading-only/frontmatter-only input, called for substantive input, and that an all-non-substantive directory produces the neutral, non-embeddable overview.tests/service/test_reindex_placeholder.py— added a case proving the new no-content marker is recognized by the reindex guard.Run:
pytest tests/storage/test_has_substantive_content.py tests/storage/test_semantic_substantive_gate.py tests/storage/test_semantic_gate_e2e.py tests/storage/test_semantic_dag_skip_files.py tests/storage/test_semantic_processor_l0_l1.py tests/service/test_reindex_placeholder.py→ 53 passed. Ruff clean; no new type-check errors.Checklist
Additional Notes
The detector is deliberately biased toward recall: the default threshold (8 weighted chars) keeps thin-but-real content (a one-line body such as
Run `make build` to compile.) while rejecting title-only files. Dropping a real file from search is worse than summarizing a thin one, andmin_substantive_charscan be raised if production shows residual hallucination on thin files. CJK content is weighted (×2.5) so a few Chinese characters clear the gate, since character count alone under-counts dense CJK text.