phase: 113_source_chip_quality
All gates green — no defects found; this pass was verification only. **Phase 113 final verification pass — report** - Verified (no code changes needed): `select_documents_tiered` cited/related tiering + `select_documents` wrapper, `TurnPlan.related_docs`, `ChatDoneEvent.related` (additive, old payloads parse), `appendRelated` UI row (`.related-doc`, never `.source-chip`), done-frame + restore-path wiring, two settings with validators, `.env.example` entries - `uv run pytest --cov=app --cov-report=term-missing` → 2422 passed, app/ coverage **99%** (>90% gate) - `uv run pytest tests/e2e/test_source_chip_quality.py -v --no-cov` (isolated) → 2 passed - Regression E2E `test_retrieval_quality.py` + `test_honest_deflection.py` + `test_chat_rag.py` + `test_sources_midstream_bug.py` → 17 passed - `uv run ruff check . && uv run pyright` → clean (0 errors); `bash .agents/validate.sh` → "validation OK" Completion criteria: 1. Single-doc question → exactly one `.source-chip` (E2E): ✅ passed 2. Weak 2nd doc only in de-emphasized related row, never `.source-chip` (unit + E2E): ✅ passed 3. Deflected turn → zero citation chips, weak hits in related row: ✅ passed 4. Full suite green, coverage >90%, isolated E2E green, lint/types clean: ✅ passed 5. `--no-gpg-sign` commit + phase dir move: left to harness per pass rules (task files already in `complete/`) No deviations. Next pending phase: `114_embed_question_length`.
This commit is contained in:
@@ -443,6 +443,116 @@ def test_off_topic_question_deflects_honestly(client, db, seeded_kb: FakeRagLLM)
|
||||
assert row.sources
|
||||
|
||||
|
||||
def test_done_frame_carries_related_tier_on_grounded_turn(
|
||||
client, db, seeded_kb: FakeRagLLM
|
||||
) -> None:
|
||||
"""Phase 113 (LOCKED A4): a grounded turn's done frame carries the
|
||||
related tier — the ranked docs beyond the cited ceiling, capped at
|
||||
``related_max_docs`` (2), disjoint from the cited list. The durable
|
||||
record keeps the FULL retrieval (cited + related, LOCKED A3)."""
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: seeded_kb
|
||||
try:
|
||||
_, _, frames = _stream_chat(client, QUESTION)
|
||||
finally:
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
|
||||
done = frames[-1]
|
||||
assert done["deflected"] is False
|
||||
sources = {(s["source"], s["path"]) for s in done["sources"]}
|
||||
related = done["related"]
|
||||
assert related, "the 2nd-and-lower scored docs ride the related tier"
|
||||
assert len(related) <= get_settings().related_max_docs
|
||||
# The related tier never overlaps the cited list (the dedupe is by
|
||||
# (source, path) — the same pattern as the cited docs).
|
||||
related_keys = {(s["source"], s["path"]) for s in related}
|
||||
assert sources.isdisjoint(related_keys)
|
||||
# Rank order: the cited top-2 are the kubernetes doc and the template;
|
||||
# the next ranked doc is the ssh aliases file.
|
||||
assert related[0]["path"] == "homelab/ssh/ssh_aliases.txt"
|
||||
# Every ref carries the chip identity fields (the UI row reuses them).
|
||||
assert all(s["title"] for s in related)
|
||||
|
||||
# Durable record: the full retrieval (cited + related) is logged.
|
||||
row = db.scalars(select(QueryLog)).one()
|
||||
assert "docs/homelab/ssh/ssh_aliases.txt" in row.sources
|
||||
assert "docs/homelab/kubernetes.md" in row.sources
|
||||
|
||||
|
||||
def test_deflected_done_frame_carries_weak_hits_in_related(
|
||||
client, db, seeded_kb: FakeRagLLM, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Phase 113: on a deflected turn nothing clears the bar — the cited
|
||||
tier is empty (done.sources stays [], the phase-112 contract) and the
|
||||
weak hits fall to the related tier (their visibility home). The
|
||||
durable record still carries the retrieval (LOCKED A3)."""
|
||||
monkeypatch.setenv("BOR_SOURCE_USEFULNESS_FLOOR", "0.20")
|
||||
get_settings.cache_clear()
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: seeded_kb
|
||||
try:
|
||||
_, _, frames = _stream_chat(client, OFF_TOPIC)
|
||||
done = frames[-1]
|
||||
assert done["deflected"] is True
|
||||
assert done["sources"] == [] # a deflected answer cites nothing
|
||||
# The weak hits (the sourdough question's best mock cosines are
|
||||
# ~0.11/0.04 — both below the 0.20 bar) ride the related tier, in
|
||||
# rank order, capped at related_max_docs.
|
||||
related = done["related"]
|
||||
assert len(related) <= get_settings().related_max_docs
|
||||
assert [s["path"] for s in related][:2] == [
|
||||
"deployments/new-service.md",
|
||||
"homelab/quadlet/lan.network",
|
||||
]
|
||||
assert all(s["title"] for s in related)
|
||||
assert done["suggestions"] # the "Maybe try" chips are unchanged
|
||||
|
||||
# Durable record: the weak retrieval stays logged for tuning (A3).
|
||||
row = db.scalars(select(QueryLog)).one()
|
||||
assert row.deflected is True
|
||||
assert row.sources # the weak-hit paths, for threshold tuning
|
||||
finally:
|
||||
# The cache clear is LAST — an assertion that calls get_settings()
|
||||
# after the clear would re-populate the lru_cache with the
|
||||
# monkeypatched value and leak it into the next test.
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
get_settings.cache_clear()
|
||||
|
||||
|
||||
def test_related_doc_read_by_agent_is_cited_not_related(
|
||||
client, db, seeded_kb: FakeRagLLM
|
||||
) -> None:
|
||||
"""Phase 113 × phase 37: an agent-read doc is a citation by definition
|
||||
— when the agent ``read``s a doc that would otherwise ride the related
|
||||
tier, it joins done.sources (deduped, last) and is EXCLUDED from
|
||||
done.related (a "nearby doc" that was actually used must not read as
|
||||
nearby)."""
|
||||
scripted = FakeRagLLM(
|
||||
tool_script=[
|
||||
[
|
||||
ToolCallPiece(
|
||||
id="call_1",
|
||||
name="read",
|
||||
arguments={"path": "docs/homelab/ssh/ssh_aliases.txt"},
|
||||
)
|
||||
]
|
||||
]
|
||||
)
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: scripted
|
||||
try:
|
||||
_, _, frames = _stream_chat(client, QUESTION)
|
||||
finally:
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
|
||||
done = frames[-1]
|
||||
assert done["deflected"] is False
|
||||
sources = [(s["source"], s["path"]) for s in done["sources"]]
|
||||
assert sources[-1] == ("docs", "homelab/ssh/ssh_aliases.txt") # read ⇒ cited
|
||||
related = [(s["source"], s["path"]) for s in done["related"]]
|
||||
assert ("docs", "homelab/ssh/ssh_aliases.txt") not in related
|
||||
assert set(sources).isdisjoint(set(related))
|
||||
# The OTHER related-tier doc (gitlab, rank 4) stays in the tier.
|
||||
assert ("docs", "homelab/container_gitlab/gitlab.md") in related
|
||||
|
||||
|
||||
def test_keyword_question_grounded_by_lexical_hit_despite_weak_cosine(
|
||||
client, db, seeded_kb: FakeRagLLM,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
@@ -462,26 +572,28 @@ def test_keyword_question_grounded_by_lexical_hit_despite_weak_cosine(
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: seeded_kb
|
||||
try:
|
||||
_, _, frames = _stream_chat(client, "How does kafkabridge work?")
|
||||
done = frames[-1]
|
||||
assert done["type"] == "done"
|
||||
assert done["deflected"] is False # weak cosine, but a lexical hit
|
||||
assert done["suggestions"] == []
|
||||
sources = done["sources"]
|
||||
assert sources and sources[0]["path"] == "homelab/networking/static-dns.json"
|
||||
|
||||
(system, _user) = seeded_kb.seen_messages[0][0], seeded_kb.seen_messages[0][1]
|
||||
assert "<relevance>HIGH</relevance>" in system["content"] # grounded prompt
|
||||
|
||||
row = db.scalars(select(QueryLog)).one()
|
||||
assert row.deflected is False
|
||||
assert row.top_score < get_settings().relevance_threshold # weak vector score
|
||||
assert (row.fts_hits or 0) >= 1 # …and it is the FTS hit that grounds it
|
||||
assert "docs/homelab/networking/static-dns.json" in row.sources
|
||||
finally:
|
||||
# The cache clear is LAST — an assertion that calls get_settings()
|
||||
# after the clear would re-populate the lru_cache with the
|
||||
# monkeypatched value and leak it into the next test.
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
get_settings.cache_clear()
|
||||
|
||||
done = frames[-1]
|
||||
assert done["type"] == "done"
|
||||
assert done["deflected"] is False # weak cosine, but a lexical hit
|
||||
assert done["suggestions"] == []
|
||||
sources = done["sources"]
|
||||
assert sources and sources[0]["path"] == "homelab/networking/static-dns.json"
|
||||
|
||||
(system, _user) = seeded_kb.seen_messages[0][0], seeded_kb.seen_messages[0][1]
|
||||
assert "<relevance>HIGH</relevance>" in system["content"] # grounded prompt
|
||||
|
||||
row = db.scalars(select(QueryLog)).one()
|
||||
assert row.deflected is False
|
||||
assert row.top_score < get_settings().relevance_threshold # weak vector score
|
||||
assert (row.fts_hits or 0) >= 1 # …and it is the FTS hit that grounds it
|
||||
assert "docs/homelab/networking/static-dns.json" in row.sources
|
||||
|
||||
|
||||
def test_chat_empty_kb_streams_empty_sources(client, db) -> None:
|
||||
db.execute(text("TRUNCATE chunks, documents, query_log"))
|
||||
@@ -1875,6 +1987,9 @@ def test_done_event_serializes_column_maximum_source_refs() -> None:
|
||||
"type": "done",
|
||||
"deflected": False,
|
||||
"sources": [{"source": "s" * 120, "path": "p" * 1000, "title": "t" * 500}],
|
||||
# Phase 113: the additive related tier defaults to [] (the
|
||||
# key is always present on new frames; old clients ignore it).
|
||||
"related": [],
|
||||
"suggestions": [],
|
||||
}
|
||||
# The caps sit exactly ON the column maxima: one over any of them
|
||||
@@ -1886,3 +2001,29 @@ def test_done_event_serializes_column_maximum_source_refs() -> None:
|
||||
SourceRef(source="s" * 120, path="p" * 1001, title="t" * 500)
|
||||
with pytest.raises(ValidationError):
|
||||
SourceRef(source="s" * 120, path="p" * 1000, title="t" * 501)
|
||||
|
||||
|
||||
def test_done_event_related_defaults_empty_and_old_payload_parses() -> None:
|
||||
"""Phase 113 back-compat pin: ``related`` defaults to ``[]`` — a
|
||||
pre-phase-113 done frame (no ``related`` key) still parses, and a
|
||||
frame with the field round-trips it (PLAN §4: old clients ignore
|
||||
unknown fields, so the field is additive in both directions)."""
|
||||
old_payload = {
|
||||
"type": "done",
|
||||
"deflected": True,
|
||||
"sources": [],
|
||||
"suggestions": ["Maybe try X?"],
|
||||
}
|
||||
event = ChatDoneEvent(**old_payload)
|
||||
assert event.related == []
|
||||
assert event.model_dump() == {**old_payload, "related": []}
|
||||
|
||||
new_payload = {
|
||||
"deflected": False,
|
||||
"sources": [SourceRef(source="docs", path="a.md", title="A")],
|
||||
"related": [SourceRef(source="docs", path="b.md", title="B")],
|
||||
"suggestions": [],
|
||||
}
|
||||
dumped = ChatDoneEvent(**new_payload).model_dump()
|
||||
assert [r["path"] for r in dumped["related"]] == ["b.md"]
|
||||
assert [r["path"] for r in dumped["sources"]] == ["a.md"]
|
||||
|
||||
Reference in New Issue
Block a user