test(w7-task#11): scaffold Wave 7 e2e narrative — 9-step skeleton, bodies pending task #8 wiring#1760
Conversation
…dies pending task #8 wiring Per architect ratify msg=a0ba75da + huangheng CR rationale msg=0b48af2b, task #11 is the integration safety net for the three pieces of wiring that task #8 introduces: 1. ``LineageGraphStoreWithAliasRedirect`` decorator wrap in ``worker_factory._build_lineage_graph_store``. 2. REST ``POST /collections/{cid}/graphs/nodes/merge`` cutover from legacy ``GraphIndexService.merge_entities`` to the new ``GraphCurationService.merge_entities``. 3. ``retrieval/pipeline.py:_graph_search`` cutover to vector recall. Unit tests can prove each wiring CALL exists; only the end-to-end narrative validates the *behaviour*. Per huangheng's CR rationale: "grep 只能 catch wiring 是否存在, 不能 catch wiring 是否 produce 期望行为". This PR ships only the scaffold: * File ``tests/integration/test_w7_e2e_graph_recall_and_merge.py`` with 9 narrative test functions, each documenting its step shape. * Module-level ``pytestmark`` skips the file behind two gates: - ``_TASK8_WIRING_LANDED = False`` flips True alongside the task #8 PR (or in this PR's own follow-up commit) to enable bodies. - ``RUN_W7_E2E_NARRATIVE=1`` env gate so local-dev pytest stays fast; CI Wave 7 lane sets it once task #8 wiring is alive (mirrors the Wave 4 ``test_full_indexing_pipeline.py`` Layer 2 gate pattern). * ``pytest -v`` collects 9 tests, all skip cleanly. Sequence (per huangheng + architect): task #8 merge → flip ``_TASK8_WIRING_LANDED = True`` + fill bodies + run Layer 2 → push final PR → merge BEFORE task #10 close-out (so a regression introduced by deleting legacy is caught while rollback is still cheap). Step-9 fold-in (per architect msg=a0ba75da): failure-mode integration — simulate compactor LLM down → re-sync, verify Wave 6 graceful degrade preserved. Step-8 W8-3 trigger pin (per huangheng msg=0b48af2b): ``GET /collections/{cid}/graphs/nodes/{alicia_id}`` returns 404 today (read-side alias resolution deferred to Wave 8 W8-3). When W8-3 ships the assertion flips to "200 with Alice payload" — the trigger condition is physically pinned in the repo.
14ea3d5 to
4057084
Compare
…+ post-#1762 lint sweep) Task #8 PR #1762 merged 2026-04-28 (commit 08d9d3b). Updates to this scaffold + a small repo-wide lint sweep that pre-commit caught. This PR's substantive change (single test file): * ``_TASK8_WIRING_LANDED`` flipped True; module-level wiring skip removed. The file now gates only on ``RUN_W7_E2E_NARRATIVE=1`` (Layer 2 env gate) so local-dev pytest stays fast while CI Wave 7 lane can run bodies. * Per-step skip messages rewritten as concrete API pointers — each step names the exact merged surface its body must call (REST path, service method, repository class), so the body-fill follow-up can grep straight to the right spot. Bodies still pending implementation — they need a running stack (Postgres + Qdrant + Redis + ES + LLM provider keys). PR remains draft until bodies are filled + Layer 2 has run green at least once. Bundled lint cleanup (ruff format only, zero behavior change): * aperag/mcp/server.py — import block re-sort (post #1759 squash ordering drift). * aperag/domains/knowledge_graph/service.py * aperag/graph_curation/lineage_merge.py * tests/unit_test/mcp/test_graph_tools.py * tests/unit_test/service/test_graph_search_service_layer.py The four ``aperag/`` + ``tests/unit_test/`` files above all came in via the PR #1762 squash merge moments ago and trip ``ruff format --check``. Bundling here so this PR can land cleanly through pre-commit; the alternative is a separate "format-only" PR for code that just merged, which is more PR overhead than value.
Per architect msg=fa045579 routing — chenyexuan owns body fill on 冬柏's branch. Bodies exercise the three task #8 wiring points behaviourally so a regression that survives unit-level grep is caught here while rollback is still cheap. Approach * Service-layer + DB-direct narrative (not HTTP through FastAPI). The HTTP shape is pinned by the task #7 / task #8 route unit tests; narrative-correctness lives in the service-layer behaviour the routes delegate to (``GraphService.merge_entities``, ``GraphService.get_entity_detail``, ``GraphSearchService.search_entities``, ``LineageGraphStoreWithAliasRedirect``, ``AliasMapRepository.resolve_canonical``). This keeps the test fixture-light: no live FastAPI, no auth bootstrap, no Collection ORM seeding — just module-scoped store / decorator / repo fixtures bound to a synthetic collection_id. * Pre-extract entities (Alice / Bob / Acme / Alicia) via direct ``upsert_entity_with_lineage`` so the narrative does not depend on a non-deterministic LLM extractor's output. The wiring under test is the storage + vector + alias paths, not the extractor prompt. * Module-scoped state via the lineage store + alias_map (production data plane) so step N+1 sees step N's side-effects without Python globals. Step coverage step 1 seed Alice / Bob / Acme via raw inner store; assert ``get_entity`` returns them post-write. step 2 build the per-collection ``GraphSearchService`` factory; re-assert step 1 entities survived (snapshot-diff invariant — Wave 4 §C.3). step 3 ``search_entities("Alice", top_k=5)`` shape contract: list of ``EntityWithLineage`` (graceful-degrade on empty per Wave 6 keyword-path convention). step 4 call ``GraphService.merge_entities(target="Alice", sources=["Alicia"])`` (the function the REST route now delegates to). Asserts backward-compat shape including ``edges_redirected/edges_collapsed == 0`` (Wave 7 §K.12 invariant #9 / cuiwenbo schema diff lock). step 5 ``AliasMapRepository.resolve_canonical("Alicia")`` → ``"Alice"``. Self-canonical case ``Alice → Alice`` also asserted. step 6 the critical inseparability gate (wiring #1). Re-extract ``Alicia`` through the *decorated* store. Assert ``get_entity("Alicia") is None`` (silent redirect) and the new lineage member shows up under ``"Alice"``. step 7 ``search_entities("Alice")`` round-trip post-merge — alias name MUST NOT leak into the recall result (the alias is a payload-redirected write, not a separate vector point). step 8 W8-3 trigger pin. ``GraphService.get_entity_detail ("Alicia")`` returns ``None`` today because read-side alias resolution is deferred to Wave 8. The assertion message documents the flip when W8-3 ships ("flip to ``assert result.name == 'Alice'``"), so the trigger condition is mechanically pinned in the repo (per architect msg=a0ba75da + huangheng msg=0b48af2b). step 9 failure-mode fold-in. Patch ``GraphIndexCompactor.compact_if_oversized`` to raise. Re-trigger the merge. Assert the merge still completes with a unified description (graceful degrade — compaction failure is non-fatal per Wave 6 ``test_w7_phase3_*_failure_non_fatal`` unit invariant). Layer 2 gating preserved — tests skip cleanly under default ``pytest`` (no ``RUN_W7_E2E_NARRATIVE=1``); CI Wave 7 lane flips it on once the e2e-http-compose stack provides Postgres / Redis / Qdrant / Elasticsearch + provider keys. Local verification * ``uv run pytest tests/integration/test_w7_e2e_graph_recall_and_merge.py --collect-only`` → 9 collected. * ``uv run pytest`` (default gate off) → 9 skipped. * ``uv run ruff check`` clean. Bodies use real production APIs (no mock-of-mock) so end-to-end verification on the e2e-http-compose lane is what 冬柏 retains for the un-draft toggle (per msg=fa045579). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lf-canonical Per @冬柏 msg=8f488513 + PM msg=a769156c review items: * **Q1 step 8 W8-3 trigger pin**: lift the future-flip instruction from a docstring comment into the ``assert ... , ""`` message so the Wave 8 implementer's traceback names the exact line to flip (``flip to: assert result is not None and result.name == 'Alice'``). Comments are easy to grep-miss; assert messages travel with the failure. * **Q3 step 5 self-canonical case**: drop the ``Alice → Alice`` resolve_canonical assertion. It is degenerate corner-case coverage that already lives in unit-level ``test_alias_map_resolve_canonical*``; carrying it on the e2e narrative dilutes the merge-journey storyline without adding integration value. Q2 (step 9 monkeypatch target ``GraphIndexCompactor.compact_if_oversized``) acknowledged unchanged — already aligned with the unit-level ``test_w7_phase3_*_failure_non_fatal`` pattern. Local verify: 9 collected + 9 skipped under default gate; ruff clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
earayu
left a comment
There was a problem hiding this comment.
🟢 LGTM ✅ (huangheng narrative-correctness CR, per spec §K.12.11) — GitHub 不允许同账号 approve;verdict = ready to merge
PR #1760 是 Wave 7 e2e narrative validation 物理交付 — 9-step happy path + W8-3 trigger pin + failure-mode fold-in,narrative-correctness 是核心 hard-gate(不同于 unit PR 12-invariant cross-check,e2e 验证 happy path 行为级正确而非 per-invariant 落点)。chenyexuan body fill + 冬柏 step 8/9 review 双方对齐质量高。
Narrative-correctness 9-step verification
| Step | Coverage | Wave 7 invariant 行为级 verify |
|---|---|---|
| 1 seed initial entities (kg.jsonl 3 entity + 2 relation) | inner_store fixture seed | L1 graph data 写入 (invariant #1 行为) |
| 2 Compactor + vector writer Phase 3 | Phase 3 写入路径 | wiring #1 worker_factory decorator wrap + #3 Compactor 顺序 + #4 vector store via Adaptor + #5 payload filter + #6 uuid5 + #7 snapshot-diff (整 task #3 wiring) |
3 search_entities("Alice") 返 Alice |
retrieval pipeline cutover | wiring #3 task #8 _graph_search cutover behavior 行为级 verify ✅ |
4 REST /graphs/nodes/merge target=Alice sources=[Alicia] |
service.merge_entities → LineageEntityMerger | wiring #2 task #8 REST route swap behavior 行为级 verify ✅ |
| 5 alias_map row persisted (Alicia → Alice) | AliasMapRepository state verify | invariant #7 alias_map persist + 行为级 cycle reject + transitive flatten (task #6 内部) |
| 6 silent alias redirect on indexer-side upsert | re-add doc 含 Alicia → 验证 upsert 重定向到 Alice 行 | inseparability gate 行为级 verify (invariant #3 — task #6 PR #1758 + task #8 wiring #1 联合行为) ⭐⭐⭐ Wave 7 核心价值物理 hit |
| 7 re-search after merge returns canonical | post-merge query consistency | retrieval + merger end-to-end consistency |
8 W8-3 trigger pin: get_entity_detail("Alicia") 返 404 today |
future-proof flip-test | Wave 8 W8-3 ship 时此 test fail → flip mechanically;assert message 含 explicit "flip to: ..." 指引 ✅ huangheng + 冬柏 review feedback applied |
| 9 failure-mode: compactor LLM down → graceful degrade | monkeypatch GraphIndexCompactor.compact_if_oversized raise |
merger orchestration 走 fallback (unified description + skip compaction);与 unit test_w7_phase3_*_failure_non_fatal 同款 monkeypatch target,单测 + 集成 narrative 双层守护 |
关键 wiring 行为级 verify:step 3 (wiring #3) + step 4 (wiring #2) + step 6 (wiring #1 inseparability) — task #8 三 wiring 物理 alive 的硬证据。grep-zero (task #10) 只能验存在,narrative 验行为,互补完整。
review feedback 落地 verification
chenyexuan apply (commit 85cd33b) |
冬柏 feedback | 落地 |
|---|---|---|
| Q1 step 8 W8-3 assert message future-flip | 写进 assert message 而非仅注释 | line 538-543 explicit "flip to: assert result is not None and result.name == 'Alice'" ✅ Wave 8 implementer traceback 直接 mechanical 修复 |
| Q2 step 9 monkeypatch target | GraphIndexCompactor.compact_if_oversized (与 unit pattern 同款) |
line 570-573 ✅ |
| Q3 step 5 self-canonical case 删 | over-scope | self-canonical 已 drop ✅ unit test_alias_map_resolve_canonical* 范畴 |
12-invariant cross-check (narrative scope, 大部分 n/a 但 step 行为级 implicit 验证)
| # | Invariant | This PR (narrative) |
|---|---|---|
| 1 | L1 graph data 不污染 | ✅ implicit (step 1 seed via storage view DTO;step 6 re-add doc 不污染 alias_map row) |
| 2 | L1 → L2 单向派生 | ✅ implicit (step 2 Phase 3 顺序行为级 alive) |
| 3 | Compactor 顺序 | ✅ implicit (step 2 + step 9 graceful degrade) |
| 4 | Vector store via Adaptor | ✅ implicit (step 3 search → vector store) |
| 5 | payload indexer="graph_entity" |
✅ implicit (step 3 search filter) |
| 6 | uuid5 deterministic id | ✅ implicit (step 7 re-search consistency) |
| 7 | snapshot-diff name set | n/a in narrative (task #3 unit 已 cover) |
| 8 | alias_map orphan persist | n/a (post-gc 不在 narrative scope) |
| 9 | upsert 透明 alias redirect | ✅✅✅ step 6 直接行为级 verify (inseparability gate) — narrative 核心价值 |
| 10 | DB column length cap | n/a |
| 11 | 候选检测仅写不自动合并 (D-3) | n/a in narrative (task #4 unit 已 cover) |
| 12 | 命名 grep-zero LightRAG | ✅ grep -i lightrag tests/integration/test_w7_e2e_graph_recall_and_merge.py → 0 hits |
4-pattern pre-check matrix (PR body 已述)
PR description 提到 bodies-filled update 时 paste,当前 PR body 是 scaffold-stage description;body 已 fill 但 description 未 update。建议 chenyexuan / 冬柏 update PR body 加完整 4-pattern paste(不阻塞,可 fix-forward)。
simple-stable 4-guardrail
- #1 不无限扩范围: 1 file e2e narrative + 9 step happy path + 1 W8-3 pin + 1 failure-mode;不动 production code ✅
- #2 尽快上线: chenyexuan body fill + 冬柏 review iter ~30min total bandwidth,scaffold 与 #8 parallel 不阻塞 ✅
- #3 简单稳定: e2e narrative 是 ship 信心硬保证,比 unit 抓 integration drift 早 ✅
- #4 私有化部署免维护: 自 contained,env gate
RUN_W7_E2E_NARRATIVE=1防本地误跑 ✅
CI gate
PR description 标注 "CI Wave 7 lane 跑通后 merge"。冬柏 retain merge 权 - architect ratify after CI 绿即可 ship in #1765 同期 / 之后。
修完会 LGTM 的清单
实际上已经可以 merge ✅。1 个 minor: PR body 是 scaffold-stage description,bodies + 4-pattern hard-gate paste 在 PR body 里没 update(fix-forward 都行)。
@chenyexuan body 实施 + Q1 W8-3 assert message future-flip + Q2/Q3 review apply 都到位。
@冬柏 step 8/9 final review + un-draft + ruff format commit 都到位 + W8-3 trigger condition 物理钉死在仓库是高质量产物。👍
@符炫炜 architect ratify final 可 merge after CI 绿(同账号限制)。
@不穷 Wave 7 close-out 序列:#1760 (本) || #1765 (Bryce 力 fold-in 中) → architect final review → @earayu2 sign-off。
Wave 7 close-out narrative 物理交付意义
step 6 inseparability gate 行为级 verify 是 Wave 7 核心价值"alias redirect 在 indexer write-time 透明完成"的物理证据 — task #6 PR #1758 (decorator) + task #8 PR #1762 (worker_factory wrap) 联合行为通过 narrative 验证而非仅 unit grep。这是 ship-confidence 的硬保证。
Wave 7 真正 close-out 路径:#1760 + #1765 (fold-in #1763) → architect final review → 全 11 PR + 2 spec PR + 1 test infra → @earayu2 sign-off → Wave 8 candidate W8-1~W8-6 交付下次 wave kickoff。
…gger pin (#1769) Extends ``LineageGraphStoreWithAliasRedirect`` to also intercept read-path methods that take entity names as input, so MCP / REST / curation surfaces resolve alias names to their canonical before hitting the inner store. Closes the W8-3 trigger pin Wave 7 task #11 left open (huangheng W8-3 sediment from task #7 PR CR msg=55974e1e). Implementation -------------- Per architect ratify msg=486fc794 + msg=ee7a3xxx (decorator extension, Option (a)): - ``get_entity(name)`` — resolve name through alias map before ``inner.get_entity``. - ``get_relation(source, target, type)`` — resolve both endpoints symmetrically (mirror write-side ``upsert_relation_with_lineage``). - ``expand_neighbors_n_hops(entity_names, hops)`` — resolve every anchor via ``asyncio.gather``; dedupe so callers passing both alias and canonical (``["Alicia", "Alice"]``) don't double-walk. Methods that do NOT redirect (documented in module docstring): ``query_entities_by_keyword`` (input is a keyword, not a name), ``list_entity_labels`` (returns labels, not names), ``find_*_with_lineage`` (filters by document_id), ``gc_*_if_orphan`` / ``delete_*`` (canonical-row-only operations). Task #11 W8-3 trigger pin flip ------------------------------- ``tests/integration/test_w7_e2e_graph_recall_and_merge.py`` step 8 (``test_step8_get_entity_detail_alias_returns_404_w8_3_pin``) was the W8-3 trigger pin: it asserted ``result is None`` to encode the deferred behaviour as a future-flip marker. This PR flips it to assert ``result.name == 'Alice'`` and renames it to ``test_step8_get_entity_detail_alias_returns_canonical_w8_3_landed``. The new assert messages document the redirect as the cause if it regresses, mirroring the Wave 7 task #11 future-flip pattern in reverse. §K.12 invariant cross-check (12-item) ------------------------------------- | # | Invariant | This PR | |---|---|---| | 1 | L1 graph data 不污染 | ✅ read-only intercept; never writes | | 2 | L1 → L2 单向派生 | n/a (read-only) | | 3 | Compactor 在 sync 末尾 | n/a | | 4 | Vector store via abstraction | n/a (alias_repo is SQL) | | 5 | payload indexer filter | n/a | | 6 | uuid5 deterministic id | n/a | | 7 | snapshot-diff via lineage entity name set | n/a | | 8 | alias_map orphan persist | ✅ read redirect surfaces canonical even when GC'd | | 9 | upsert_entity_with_lineage transparent alias redirect | ✅ extended symmetrically to reads | | 10 | DB column length cap | n/a | | 11 | 候选检测仅写不自动合并 (D-3) | ✅ read-only | | 12 | 命名 grep-zero LightRAG | ✅ 0 hits in new code | Test plan --------- 12/12 cases pass (`uv run pytest tests/unit_test/indexing/test_alias_redirect_store.py -v`): 5 new read-side cases: - ``test_get_entity_after_alias_returns_canonical_data`` - ``test_get_entity_no_alias_passes_through_unchanged`` - ``test_get_relation_redirects_both_endpoints`` - ``test_get_relation_redirects_only_one_endpoint`` - ``test_expand_neighbors_redirects_alias_anchor`` - ``test_expand_neighbors_dedupes_alias_and_canonical_in_anchors`` - ``test_expand_neighbors_empty_seeds_passes_through`` Plus 4 pre-existing write-side cases retained, and 1 renamed passthrough test verifying both groups passthrough byte-for-byte under empty alias_map. The integration test step 8 flip is also covered (assertion changed from ``is None`` to ``not None and .name == 'Alice'``); narrative test in PR #1760 will validate the e2e wiring once redeployed. `ruff check` + `ruff format --check` clean.
Wave 7 task #11 — end-to-end narrative validation. The integration safety net for the three task #8 wiring points (worker_factory decorator wrap, REST merge route cutover, retrieval pipeline cutover). Pairs with PR #1762 wiring landing.
Summary
9-step narrative test in
tests/integration/test_w7_e2e_graph_recall_and_merge.pyvalidates Wave 7 core value (vector recall + alias inseparability + user-driven merge) at the service-layer + DB level. Layer 2 gated byRUN_W7_E2E_NARRATIVE=1; default pytest stays fast (9 collected + 9 skipped).9-step coverage
upsert_entity_with_lineageGraphSearchServicefactory; assert snapshot-diff invariantGraphSearchService.search_entities("Alice")→ list ofEntityWithLineageGraphService.merge_entities(target=Alice, sources=[Alicia])→ backward-compat shape (edges_*=0)AliasMapRepository.resolve_canonical("Alicia")→"Alice"get_entity("Alicia") is None+ new lineage member shows up under Alicesearch_entities("Alice")post-merge — assert alias name does NOT leakGraphService.get_entity_detail("Alicia")returnsNone(read path bypasses alias_map). Assert message includes future-flip instruction so Wave 8 implementer's traceback names the line to flipGraphIndexCompactor.compact_if_oversizedto raise; merge still completes (Wave 6 graceful degrade)test_w7_phase3_*_failure_non_fatalintegration counterpartApproach
upsert_entity_with_lineageto avoid LLM extractor non-determinism. The wiring under test is storage / vector / alias paths, not the extractor prompt.inner_storeraw +decorated_storealias-redirect-wrapped +alias_repo) bound to a synthetic collection_id; module-level state lives on the production data plane (lineage store + alias_map), not Python globals.§K.12 12-invariant cross-check
EntityWithLineage(storage view), kg.jsonl raw not touchedGraphSearchServicefactory composes Adaptor; step 3 exercises it4-pattern pre-check matrix
rg "from aperag.domains.knowledge_graph.graphindex" tests/integration/test_w7_e2e_graph_recall_and_merge.py→ 0 matches.upsert_entity_with_lineage/get_entity/search_entities/GraphService.merge_entities/AliasMapRepository.resolve_canonical/GraphService.get_entity_detail— all post-Wave 7 surfaces.resolve_canonical) and step 6 (decorator's transparent redirect on upsert).GraphSearchService; step 6 reads through_build_lineage_graph_store(decorated) vs_build_lineage_graph_store_inner(raw).simple-stable 4-guardrail
RUN_W7_E2E_NARRATIVE=1and runs as-is.assertper step).W8-3 trigger condition pinned in step 8
When Wave 8 W8-3 ships read-side alias resolution,
GraphService.get_entity_detail("Alicia")will start returning a row instead ofNone. This test fails; its assert message names the exact flip:The test IS the W8-3 trigger condition pinned in the repo.
Local verification
Test plan
RUN_W7_E2E_NARRATIVE=1) — 9 tests pass against running stack (Postgres + Redis + Qdrant + Elasticsearch + provider keys)🤖 Generated with Claude Code