phase: 141_read_range_dedupe_fix
All gates green and verified. Final report: **Phase 141 final verification pass — complete** - **Verified (tasks 01–04 already in `complete/`):** `AgentHolder.read_coverage` span recording (all 4 read outcomes, refusals record nothing); coverage dedupe matrix (range read of uncovered lines proceeds, fully-covered re-read → `ALREADY_IN_CONTEXT` with a now-true claim, partial plain re-read → `REREAD_PARTIAL` naming merged spans, `REREAD_TRUNCATED` retired, fast path retired per task 02); `TOOLS_SECTION` re-read sentence re-cut to coverage-true form (prompt lock re-pinned); owner-scenario integration repro (pure + truncated variants) and mock `read two ranges` flow + dedicated E2E — all match the task specs exactly; no code defects found. - **Tests/lint/coverage:** `uv run pytest --cov=app --cov-report=term-missing` → exit 0, TOTAL 99% (5498 stmts, `app/rag/agent.py` 99%) — run twice, both green; `uv run pytest tests/e2e/test_read_range_dedupe.py -v --no-cov` → 2 passed in isolation; `uv run ruff check . && uv run pyright` → All checks passed / 0 errors. - **Completion criteria:** (1) two different ranges both reach the model, re-covered re-read refused truthfully — ✅ (integration + E2E); (2) partial plain re-read gets the `offset`/`limit` teaching, false claim gone both directions — ✅; (3) plain-read + phase-131 result shapes byte-identical, existing pins pass unchanged — ✅; (4) all gate commands green, >90% — ✅; (5) no behavior change in completed phases (grep/ls, truncation notice, `read_docs`/`read_truncations` signals) — ✅ (whole project suite green). - **Documentation:** README.md — fixed pre-existing phase-140 gap: `BOR_PDF_PICTORIAL` / `BOR_PDF_PICTORIAL_MIN_CHARS` were in `.env.example`/Settings but undocumented (README claims every `.env.example` var is covered); added the pictorial-PDF-pass bullet to the PDF section + two config-reference rows. AGENTS.md — verified consistent (rules, quick-ref commands, phase refs all check out) — no changes needed. - **Notable:** overview's "fast path survives for holder-state rows" note is superseded by task 02's authoritative decision (every dedupe row resolves — "full" needs the true line count); implementation follows task 02. No phase files touched; changes left in the working tree for the harness commit. - **Next pending phase:** `142_kb_overview_freshness`.
This commit is contained in:
+276
-5
@@ -644,17 +644,76 @@ Implements just enough of the aipi surface:
|
||||
N`` extent) skipped, first 80 chars (the phase-94 read-echo
|
||||
convention — BOTH ranges reached the model's context). NOTE:
|
||||
for the second read to PROCEED (not be refused
|
||||
``ALREADY_IN_CONTEXT``) the document must already be in the
|
||||
agent's truncated set — the story suite boots the app under
|
||||
test with a ``BOR_READ_MAX_CHARS`` small enough that the
|
||||
FIRST (20-line) range slice truncates, which is exactly the
|
||||
large-document premise of the owner's confusion.
|
||||
``ALREADY_IN_CONTEXT``) the document was required to already
|
||||
be in the agent's truncated set — the story suite boots the
|
||||
app under test with a ``BOR_READ_MAX_CHARS`` small enough
|
||||
that the FIRST (20-line) range slice truncates, which is
|
||||
exactly the large-document premise of the owner's confusion.
|
||||
Phase 141 RETIRED that premise: the coverage-tracked dedupe
|
||||
proceeds with a second range read whenever its lines are not
|
||||
fully covered — the suite's small cap stays as the
|
||||
large-document premise, but is no longer LOAD-BEARING for the
|
||||
second read to proceed.
|
||||
Checked BEFORE the plain ``TOOLS_TRIGGER`` flow (disjoint trigger
|
||||
phrases — the phase-71/72/94/118/131 ordering convention; the
|
||||
trigger needs the ``<tools>`` section, so deflected turns never
|
||||
hit it); verified 2026-09-30: no existing E2E question or
|
||||
fixture file contains the phrase, so every other suite is
|
||||
unaffected.
|
||||
- user message containing ``read two ranges``
|
||||
(``TWO_RANGES_TRIGGER``, phase 141 task 04 — the read
|
||||
range-dedupe fix's dedicated story suite
|
||||
``tests/e2e/test_read_range_dedupe.py``) **and** the system
|
||||
prompt carries the ``<tools>`` section -> the owner's stuck
|
||||
scenario (TODO.md L3), end to end: the model range-reads ONE
|
||||
document in TWO different 100-line ranges — pre-phase the
|
||||
second read was refused ``ALREADY_IN_CONTEXT`` ("the document
|
||||
is already in my context, but I haven't actually seen the full
|
||||
text") while ONLY the first slice was in the prompt; the
|
||||
coverage-tracked server proceeds with it, and the answer
|
||||
echoes the SECOND range's content (the marker line lives in
|
||||
range B, so an echo of it proves the slice reached the model).
|
||||
The question carries its own target after the colon (``read
|
||||
two ranges: read source/path`` — the question-carried-call
|
||||
convention of ``READ_CAP_TRIGGER`` / ``SAME_FILE_TRIGGER``),
|
||||
and the step is then discriminated statelessly by the tool
|
||||
results in the messages (the house count pattern — streaming
|
||||
only):
|
||||
* request 1 (``tools`` offered, no tool results yet): a
|
||||
``tool_calls`` delta — ``read`` on the parsed target with
|
||||
``offset`` = ``TWO_RANGES_FIRST_OFFSET`` (1) and ``limit``
|
||||
= ``TWO_RANGES_FIRST_LIMIT`` (100) (id ``call_0``);
|
||||
* EXACTLY ONE ``tool``-role read result is in the messages
|
||||
(content starting with the agent's ``"Document "`` prefix —
|
||||
``_READ_RESULT_PREFIX``) AND it carries the ``lines: 1-100
|
||||
of …`` extent header (the mock's own keying, asserted):
|
||||
the SECOND range read — a ``tool_calls`` delta — ``read``
|
||||
on the SAME target with ``offset`` =
|
||||
``TWO_RANGES_SECOND_OFFSET`` (101) and ``limit`` =
|
||||
``TWO_RANGES_SECOND_LIMIT`` (100) (id ``call_1``) — the
|
||||
flow only works if the second read PROCEEDS (the pre-phase
|
||||
server would have refused it);
|
||||
* TWO or more read results are in the messages and the
|
||||
SECOND carries the ``lines: 101-…`` header (asserted the
|
||||
same way — the mock's keying stays honest): the forced
|
||||
answer, byte-stable, in the existing read-echo shape —
|
||||
``Both parts read. <first 80 chars of the second read
|
||||
result's content, header lines skipped>`` (the
|
||||
:func:`_same_file_quote` convention — the RANGE reached
|
||||
the model's context);
|
||||
* a tool result is in the messages that is not a well-keyed
|
||||
read result for the step (a refusal — the second range
|
||||
read was refused, the pre-phase bug — or an unkeyed extent
|
||||
header): the deterministic broken-premise echo — the
|
||||
offending result VERBATIM under a ``Premise broke:`` line
|
||||
— so a suite fails loudly on it (the read-cap flow's
|
||||
broken-premise convention).
|
||||
Checked BEFORE the plain ``TOOLS_TRIGGER`` flow (disjoint
|
||||
trigger phrases — the phase-71/72/94/118/131/139 ordering
|
||||
convention; the trigger needs the ``<tools>`` section, so
|
||||
deflected turns never hit it); verified 2026-10-01: no existing
|
||||
E2E question or fixture file contains the phrase, so every
|
||||
other suite is unaffected.
|
||||
|
||||
Failure injection (phase 67, LLM retry, TODO.md L3) — deterministic
|
||||
dead-endpoint behavior for the retry E2E suite (``tests/e2e/
|
||||
@@ -2465,6 +2524,149 @@ def _same_file_flow(body: dict[str, Any]) -> tuple[str, ...] | None:
|
||||
return ("read1", target)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 141 (task 04): the owner's stuck scenario — the model
|
||||
# range-reads ONE document in TWO different ranges; the
|
||||
# coverage-tracked server proceeds with the second (the pre-phase
|
||||
# pair-set dedupe refused it ``ALREADY_IN_CONTEXT`` while only the
|
||||
# first slice was in the prompt). See the module docstring.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
#: A user message containing this substring (case-insensitive) —
|
||||
#: combined with the ``<tools>`` section in the system prompt — drives
|
||||
#: the deterministic TWO-RANGES flow (the read range-dedupe fix's
|
||||
#: story suite, ``tests/e2e/test_read_range_dedupe.py`` — the
|
||||
#: owner's stuck scenario, TODO.md L3). Checked BEFORE the plain
|
||||
#: ``TOOLS_TRIGGER`` flow (disjoint trigger phrases — the
|
||||
#: phase-71/72/94/118/131/139 ordering convention); verified
|
||||
#: 2026-10-01: no existing E2E question or fixture file contains
|
||||
#: the phrase, so every other suite is unaffected.
|
||||
TWO_RANGES_TRIGGER = "read two ranges"
|
||||
|
||||
#: The scripted target in the two-ranges question (case-
|
||||
#: insensitive — the suite's question capitalizes the trigger's
|
||||
#: first letter): the verb (``read``) plus the target — a combined
|
||||
#: ``source/path``, parsed from the RAW user message so the target
|
||||
#: keeps its case (the ``_SAME_FILE_CALL_RE`` convention; the target
|
||||
#: is a ``[a-z0-9_./-]`` run, so a `` — `` flavor separator can
|
||||
#: never bleed into it).
|
||||
_TWO_RANGES_CALL_RE = re.compile(
|
||||
r"read two ranges:\s*read\s+(?P<arg>[a-z0-9_./-]+)",
|
||||
re.I,
|
||||
)
|
||||
|
||||
#: The FIRST range read's requested extent — range A (lines 1–100
|
||||
#: of the suite's 200-line fixture document).
|
||||
TWO_RANGES_FIRST_OFFSET = 1
|
||||
TWO_RANGES_FIRST_LIMIT = 100
|
||||
|
||||
#: The SECOND range read's requested extent — range B (lines 101–
|
||||
#: …): the SAME file again, a different range. Pre-phase the server
|
||||
#: refused this call ``ALREADY_IN_CONTEXT`` (the false "full text is
|
||||
#: in your prompt" claim — the owner's stuck turn); the coverage-
|
||||
#: tracked server proceeds with it (its lines are NOT covered by
|
||||
#: the first read's span).
|
||||
TWO_RANGES_SECOND_OFFSET = 101
|
||||
TWO_RANGES_SECOND_LIMIT = 100
|
||||
|
||||
|
||||
def _two_ranges_extent(content: str) -> str | None:
|
||||
"""The ``lines: a-b of N`` extent line of one read result's
|
||||
content (the ``Document …:`` header already stripped by
|
||||
:func:`_read_results`, the phase-106 D5 ``date:`` line skipped)
|
||||
— ``None`` when the result carries no extent line (a plain
|
||||
read). The flow keys its step on this line (the mock's own
|
||||
keying, asserted — a refusal or a differently-shaped result is
|
||||
the broken premise, never a step input).
|
||||
"""
|
||||
body = content
|
||||
if body.startswith("date: "):
|
||||
body = body.partition("\n")[2]
|
||||
if body.startswith("lines: "):
|
||||
return body.partition("\n")[0]
|
||||
return None
|
||||
|
||||
|
||||
def _two_ranges_flow(body: dict[str, Any]) -> tuple[str, ...] | None:
|
||||
"""Classify a phase-141 two-ranges request (see the module
|
||||
docstring). The question carries its own target (``read two
|
||||
ranges: read source/path``); the step is then discriminated
|
||||
statelessly by the tool results in the messages (the house count
|
||||
pattern — the flow only ever issues ``read`` calls, so counting
|
||||
read results is reliable):
|
||||
|
||||
* ``("answer", quote)`` — TWO or more ``tool``-role read results
|
||||
are in the messages and the SECOND carries the ``lines: 101-
|
||||
…`` extent header (asserted — the second range read landed as
|
||||
advertised): the forced answer, byte-stable, in the existing
|
||||
read-echo shape — ``Both parts read. <quote>`` with quote the
|
||||
second result's pure content (header lines skipped, first 80
|
||||
chars — the :func:`_same_file_quote` convention; the marker
|
||||
line lives in range B, so an echo of it proves the slice
|
||||
reached the model).
|
||||
* ``("read2", target)`` — EXACTLY ONE read result is in the
|
||||
messages and it carries the ``lines: 1-100 of …`` extent
|
||||
header (asserted — the first range read landed as advertised):
|
||||
the SECOND range read on the SAME target — ``offset`` =
|
||||
:data:`TWO_RANGES_SECOND_OFFSET`, ``limit`` =
|
||||
:data:`TWO_RANGES_SECOND_LIMIT` (id ``call_1``).
|
||||
* ``("read1", target)`` — ``tools`` are offered and no tool
|
||||
result is in the messages yet: the FIRST range read on the
|
||||
parsed target — ``offset`` = :data:`TWO_RANGES_FIRST_OFFSET`,
|
||||
``limit`` = :data:`TWO_RANGES_FIRST_LIMIT` (id ``call_0``).
|
||||
* ``("bad", result)`` — a tool result is in the messages that is
|
||||
not a well-keyed read result for the step (a refusal — the
|
||||
second range read was refused, the pre-phase bug — or an
|
||||
unkeyed extent header): the deterministic broken-premise echo
|
||||
— the offending result verbatim under ``Premise broke:`` (the
|
||||
read-cap flow's broken-premise convention — a suite fails
|
||||
loudly on it).
|
||||
* ``None`` — not the flow: the trigger is absent, the
|
||||
``<tools>`` section is missing (deflected turns never carry
|
||||
it), the target is unparseable, or ``tools`` are not offered
|
||||
and no tool result is in the messages yet (e.g.
|
||||
``agent_max_rounds=0``).
|
||||
"""
|
||||
user = _user(body)
|
||||
if TWO_RANGES_TRIGGER not in user.lower():
|
||||
return None
|
||||
if "<tools>" not in _system(body):
|
||||
return None
|
||||
match = _TWO_RANGES_CALL_RE.search(user)
|
||||
if match is None:
|
||||
return None
|
||||
target = match.group("arg")
|
||||
reads = _read_results(body)
|
||||
results = _tool_results(body)
|
||||
if len(reads) >= 2:
|
||||
extent = _two_ranges_extent(reads[1][1])
|
||||
if extent is None or not extent.startswith(
|
||||
f"lines: {TWO_RANGES_SECOND_OFFSET}-"
|
||||
):
|
||||
return ("bad", reads[1][1])
|
||||
return ("answer", _same_file_quote(reads[1][1]))
|
||||
if len(reads) == 1:
|
||||
if len(results) > 1:
|
||||
# A NON-read tool result after the read result — a
|
||||
# refusal (the pre-phase bug: the second range read was
|
||||
# refused ``ALREADY_IN_CONTEXT``): the premise broke —
|
||||
# echo it so a suite fails loudly.
|
||||
return ("bad", results[-1])
|
||||
extent = _two_ranges_extent(reads[0][1])
|
||||
if extent is None or not extent.startswith(
|
||||
f"lines: {TWO_RANGES_FIRST_OFFSET}-{TWO_RANGES_FIRST_LIMIT} of "
|
||||
):
|
||||
return ("bad", reads[0][1])
|
||||
return ("read2", target)
|
||||
if results:
|
||||
# A tool result that is NOT a read result before any read
|
||||
# landed — the first read was refused: the premise broke.
|
||||
return ("bad", results[-1])
|
||||
if not body.get("tools"):
|
||||
return None
|
||||
return ("read1", target)
|
||||
|
||||
|
||||
def long_answer() -> str:
|
||||
"""~900-word deterministic walkthrough (phase 11): numbered steps plus
|
||||
a unique final line that must survive the stream untruncated."""
|
||||
@@ -3773,6 +3975,75 @@ def chat_completions(body: dict[str, Any]) -> Any:
|
||||
media_type="text/event-stream",
|
||||
headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"},
|
||||
)
|
||||
# Phase 141 (task 04): the owner's stuck scenario (the read
|
||||
# range-dedupe fix's story suite,
|
||||
# tests/e2e/test_read_range_dedupe.py — TODO.md L3): the
|
||||
# model range-reads ONE document in TWO different ranges —
|
||||
# the second PROCEEDS on the coverage-tracked server (the
|
||||
# pre-phase pair-set dedupe refused it ``ALREADY_IN_
|
||||
# CONTEXT``), and the answer echoes the second range's
|
||||
# content. Checked BEFORE the plain TOOLS_TRIGGER flow
|
||||
# (disjoint trigger phrases — the phase-71/72/94/118/131/
|
||||
# 139 ordering convention; the trigger needs the ``<tools>``
|
||||
# section, so deflected turns never hit it).
|
||||
two_ranges = _two_ranges_flow(body)
|
||||
if two_ranges is not None:
|
||||
if two_ranges[0] == "read1":
|
||||
# The FIRST range read on the parsed target — range
|
||||
# A (lines 1–100 of the suite's document).
|
||||
stream = _tool_call_stream(
|
||||
"read",
|
||||
{
|
||||
"path": two_ranges[1],
|
||||
"offset": TWO_RANGES_FIRST_OFFSET,
|
||||
"limit": TWO_RANGES_FIRST_LIMIT,
|
||||
},
|
||||
"call_0",
|
||||
)
|
||||
elif two_ranges[0] == "read2":
|
||||
# The SECOND range read — the SAME file again, a
|
||||
# different range (range B, lines 101–…). It only
|
||||
# PROCEEDS on the coverage-tracked server: its lines
|
||||
# are NOT covered by the first read's span (the pre-
|
||||
# phase pair-set dedupe refused it — the owner's
|
||||
# stuck turn).
|
||||
stream = _tool_call_stream(
|
||||
"read",
|
||||
{
|
||||
"path": two_ranges[1],
|
||||
"offset": TWO_RANGES_SECOND_OFFSET,
|
||||
"limit": TWO_RANGES_SECOND_LIMIT,
|
||||
},
|
||||
"call_1",
|
||||
)
|
||||
elif two_ranges[0] == "bad":
|
||||
# The premise broke (a refusal — the second range
|
||||
# read was refused — or an unkeyed extent header):
|
||||
# echo the offending result verbatim so a suite
|
||||
# fails loudly on it (the read-cap flow's broken-
|
||||
# premise convention).
|
||||
stream = _sse_stream(
|
||||
_apply_max_tokens(
|
||||
f"Premise broke:\n{two_ranges[1]}",
|
||||
body.get("max_tokens"),
|
||||
),
|
||||
0.0,
|
||||
)
|
||||
else: # "answer" — the forced read-echo of range B
|
||||
# (the marker line lives in range B — the echo of it
|
||||
# proves the second slice reached the model).
|
||||
stream = _sse_stream(
|
||||
_apply_max_tokens(
|
||||
f"Both parts read. {two_ranges[1]}",
|
||||
body.get("max_tokens"),
|
||||
),
|
||||
0.0,
|
||||
)
|
||||
return StreamingResponse(
|
||||
stream,
|
||||
media_type="text/event-stream",
|
||||
headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"},
|
||||
)
|
||||
# Phase 124 (task 07, image chips and answers, TODO L3): the
|
||||
# scripted SHOW_IMAGE flow — the mock shows the seeded image
|
||||
# document (the ``<documents>`` block with the image-
|
||||
|
||||
Reference in New Issue
Block a user