phase: 130_chat_answer_clusters
**Phase 130 — final verification pass: 100% green, complete.** - **Verified:** chips-only row (no chip for image docs) → images-only row (`.msg-images`) → related row → `.msg-actions` last; `getActionsRow` used by all 3 buttons + stopped/failed notes; `markLastRetryable` save-re-append hack gone; in-place chip fallback on image-load failure; CSS right-align with per-button auto margins removed — implementation matches the design in every detail. - **Completion criteria:** (1) mixed citation renders chips→images→actions-last, DOM-asserted ✅ (2) all 3 buttons in one right-aligned `.msg-actions`; non-admin sees Retry-only; restore path identical ✅ (3) 404-bytes figure → plain chip, unit-pinned ✅ (4) E2E isolated ✅ (5) no behavior change (modal affordances, related tier, ARIA names) ✅ - **Tests/lint/coverage:** `uv run pytest tests/e2e/test_chat_answer_clusters.py -v --no-cov` → 4 passed; `uv run pytest --cov=app --cov-report=term-missing` → 3061 passed, **99%** app coverage; the 4 E2E files this phase modified (test_answer_images, test_image_documents, test_pdf_support, test_responsive_polish) → 22 passed in isolation; `uv run ruff check .` clean, `uv run pyright` → 0 errors. - **Docs:** README.md — verified consistent in full (page table vs routes, env-var coverage vs `.env.example`, phase-130 cluster wording already in tree); no changes needed. AGENTS.md — fixed one pre-existing inconsistency: rule 5 still cited the retired "centered 46rem chat column"; now states the 72rem container contract (phase 100). - **Decisions:** PLAN.md §7.1 still says 46rem, but it's owner-managed (phase 100 records the supersession pending owner sign-off) and out of my editable scope — AGENTS.md now cross-references the supersession. - **Next pending phase:** `131_read_line_ranges` (todo/).
This commit is contained in:
@@ -223,26 +223,35 @@ def test_append_related_row_contract() -> None:
|
||||
# ---------- task 02: wiring — the done frame and the restore path ----------
|
||||
|
||||
|
||||
def test_done_handler_calls_append_related_last() -> None:
|
||||
def test_done_handler_appends_related_above_the_actions_row() -> None:
|
||||
"""The done-frame handler calls ``appendRelated(wrap, ev.related)``
|
||||
— and appends the row LAST among the bubble's meta rows: AFTER
|
||||
``markLastRetryable()`` (the appendTuneButton / appendSaveAsDoc /
|
||||
appendRetryButton claimers all take the FIRST ``.msg-meta`` row,
|
||||
so a deflected turn with related docs — no citation row — gets its
|
||||
OWN meta row for the buttons instead of actions joining the
|
||||
related row). The call sits after the Retry claim in the branch."""
|
||||
— and appends the row BELOW the citation clusters and ABOVE the
|
||||
actions row (phase 130 task 02): the action appenders (appendTune
|
||||
Button → markLastRetryable's Retry → appendSaveAsDocButton) FOLLOW
|
||||
it in the branch, so the get-or-created ``.msg-actions`` row is the
|
||||
bubble's LAST row (the bottom-right cluster — DOM order is visual
|
||||
order). The getActionsRow scoping keeps the buttons out of the
|
||||
related row either way: a deflected turn with related docs gets its
|
||||
OWN actions row, never a button in the related row."""
|
||||
branch = _done_branch(_app_js())
|
||||
assert "appendRelated(wrap, ev.related);" in branch, (
|
||||
"the done handler must render the related tier (it arrives in "
|
||||
"ev.related — on a deflected turn ev.sources is empty and the "
|
||||
"weak hits live here, row only, zero chips)"
|
||||
)
|
||||
assert branch.index("appendRelated(wrap, ev.related);") > branch.index(
|
||||
"markLastRetryable();"
|
||||
assert branch.index("appendRelated(wrap, ev.related);") < branch.index(
|
||||
"appendTuneButton(wrap);"
|
||||
), (
|
||||
"appendRelated must run AFTER the meta-action claimers (last "
|
||||
"meta row) — a deflected turn's tune/retry buttons must land in "
|
||||
"their own row, never in the related row"
|
||||
"appendRelated must run BEFORE the action appenders — the "
|
||||
".msg-actions row is the bubble's LAST row (phase 130 task 02)"
|
||||
)
|
||||
# The insertion order inside the row: Tune → Retry → Save-as-doc
|
||||
# (markLastRetryable re-appends the Retry button between them).
|
||||
assert branch.index("appendTuneButton(wrap);") < branch.index(
|
||||
"markLastRetryable();"
|
||||
) < branch.index('appendSaveAsDocButton(wrap, finalText || acc || "…");'), (
|
||||
"the actions row's DOM order is the insertion order: Tune, "
|
||||
"Retry, Save-as-doc (phase 130 task 02)"
|
||||
)
|
||||
|
||||
|
||||
@@ -266,25 +275,30 @@ def test_done_handler_persists_related_with_the_turn() -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_restore_path_calls_append_related() -> None:
|
||||
def test_restore_path_appends_related_above_the_actions_row() -> None:
|
||||
"""The phase-14 restore path (``renderStoredMessage`` — used by
|
||||
BOTH the localStorage restore and the /?chat=<id> saved-chat boot
|
||||
load) re-renders the related row from the stored payload when it
|
||||
carries ``related``; a pre-phase record without the field
|
||||
restores exactly as today (appendRelated no-ops — no row). The
|
||||
call sits after the meta-row claimers, exactly like the live
|
||||
done path, so restored buttons never join the related row."""
|
||||
call sits BEFORE the action appenders (Tune / Save-as-doc / notes —
|
||||
plus the restore loop's after-the-loop markLastRetryable Retry),
|
||||
so the .msg-actions row is the restored bubble's LAST row too —
|
||||
identical order to the live done path (phase 130 task 02); the
|
||||
getActionsRow scoping keeps restored buttons out of the related
|
||||
row."""
|
||||
body = _function_body(_app_js(), "function renderStoredMessage(m) {")
|
||||
assert "appendRelated(wrap, m.related);" in body, (
|
||||
"the restore path must render the related tier from the stored "
|
||||
"payload (pre-phase records carry no `related` → no row, "
|
||||
"graceful)"
|
||||
)
|
||||
assert body.index("appendRelated(wrap, m.related);") > body.index(
|
||||
"appendStoppedNote(wrap);"
|
||||
assert body.index("appendRelated(wrap, m.related);") < body.index(
|
||||
"appendTuneButton(wrap)"
|
||||
), (
|
||||
"the related row appends LAST (after the claimers) on the "
|
||||
"restore path too — identical order to the live done path"
|
||||
"the related row appends BEFORE the action appenders on the "
|
||||
"restore path too (the actions row is the bubble's LAST row) — "
|
||||
"identical order to the live done path (phase 130 task 02)"
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user