phase: 132_image_upload_feedback
Build and Push Containers / build-and-push-app (push) Successful in 2m25s
Build and Push Containers / build-and-push-db (push) Successful in 17s

All verification complete. Final report:

**Phase 132 (image upload feedback) — final verification pass: 100% green**

- **Verified** all 3 tasks' implementations in the working tree: `index.html` status part (hidden-by-default, arc + "Uploading image…", pinned order), `app.js` `enterUploadState`/`exitUploadState` (entered after `attachUpload` guard pre-await, exited on BOTH resolution paths, `setUiState` untouched, idle "Send" restore byte-identical), `styles.css` `.is-uploading` rules (house `spin` keyframes, reduced-motion block, existing `:disabled` Send language), unit pins, E2E suite with route-delayed upload window.
- **Completion criteria** — (1) mid-flight spinner + "Uploading image…", remove + Send disabled with "Uploading…": ✅ E2E-asserted; (2) second Send press = no second upload (request count exactly 1): ✅; (3) success clears + turn proceeds (bubble carries image); failure clears + banner + text/attachment remain: ✅; (4) `uv run pytest tests/e2e/test_image_upload_feedback.py -v --no-cov`: ✅ 3 passed (18.5 s, in isolation); (5) no behavior change in completed phases: ✅ (full suite green, A5 path-never-bytes re-pinned in E2E).
- **Full suite**: `uv run pytest --cov=app --cov-report=term-missing` → **3108 passed**, `app/` coverage **99%** (>90% gate); `uv run ruff check .` → clean; `uv run pyright` → 0 errors.
- **Documentation**: README.md fully reviewed (setup, pages, all `.env.example` vars + every default vs `app/config.py`, tools incl. phase-131 `read(offset,limit)`, image/PDF/sync/deploy sections) and AGENTS.md fully reviewed (rules, quick-reference commands, gitignore/gpg claims) — **verified consistent; no changes needed**. Phase-132 README bullet already in the working tree; CSS `:root` contrast-note corrections (15.3:1 / 8.2:1) re-verified mathematically correct (comments only).
- **Decisions**: none — all task work was already implemented and correct; no fixes required.
- **Next pending phase**: `133_pdf_screenshot_fallback`.
This commit is contained in:
2026-09-28 13:45:31 -04:00
parent 101156bada
commit 4cb85e5baa
22 changed files with 1696 additions and 2 deletions
+437
View File
@@ -34,6 +34,19 @@ wiring (a recording fake LLM, a fake retriever, a fake DB session,
patched settings) so the whole validate → build contract runs without
a stack; the upload/serve tests point ``chat_image_dir`` at a tmp
directory (the raw-string ``expanduser`` convention).
Phase 132 (task 01, TODO L8) pins the PRE-turn uploading state in the
same chat-frontend surface: the preview strip ships a HIDDEN status
part (the house loading arc — decorative, aria-hidden — + the pinned
"Uploading image…" text), and ``handleSend``'s attachedImage branch
enters it exactly once per upload (after the ``attachUpload`` guard,
before the ``await uploadAttachedImage``) and exits it on BOTH
resolution paths (the helper never throws) — strip class
``is-uploading`` + status unhidden + remove ✕ disabled + the Send
button disabled with "Uploading…" on entry; the mirror image on exit
with the idle label restored byte-for-byte. ``setUiState`` / ``uiState``
/ the turn loader stay untouched (the upload is pre-turn; ``uiState``
stays ``idle``).
"""
from __future__ import annotations
@@ -41,6 +54,8 @@ import base64
import io
import json
import re
import shutil
import subprocess
import uuid
from collections.abc import Iterator
from datetime import UTC, datetime
@@ -1168,3 +1183,425 @@ def test_the_bad_pick_and_the_reader_failure_keep_the_banner_copy() -> None:
m = re.search(r'reader\.onerror = \(\) => \{(.*?)\n \};', js, re.S)
assert m, "the FileReader's onerror branch must exist"
assert 'showErrorBanner("Couldn\'t attach the image — try again.");' in m.group(1)
# =====================================================================
# Phase 132 (task 01, TODO L8) — the PRE-turn uploading state
# =====================================================================
def _fn(js: str, name: str) -> str:
"""The source of the first `function <name>` in js (brace balanced
— the house pin pattern from tests/unit/test_sync_button.py)."""
fn = js.find(f"function {name}")
assert fn != -1, f"{name} must be defined"
open_idx = js.find("{", fn)
depth = 0
for i in range(open_idx, len(js)):
c = js[i]
if c == "{":
depth += 1
elif c == "}":
depth -= 1
if depth == 0:
return js[fn : i + 1]
raise AssertionError(f"unbalanced braces in {name}")
def test_the_strip_ships_the_hidden_uploading_status_part() -> None:
"""The preview strip's markup gains ONE status part INSIDE the
strip — after the filename span, before the remove button —
shipping HIDDEN (the house "ships hidden" rule): the house
loading arc (the svg is decorative, aria-hidden — the arc is not
the meaning) + the pinned "Uploading image…" text (it carries the
meaning). The strip's existing parts (thumbnail, name, remove ✕
+ its aria-label) are untouched, and the thumbnail + filename stay
visible throughout the upload (the user watches THEIR image
uploading — only the status part's `hidden` moves)."""
html = _index_html()
start = html.find('<div class="attach-preview" id="attach-preview" hidden>')
assert start != -1
strip = html[start : html.find("</div>", start)]
# the status part: the exact shipped markup (hidden by default).
assert '<span class="attach-preview-status" hidden>' in strip
assert (
'<svg class="attach-preview-status-icon" aria-hidden="true"'
' viewBox="0 0 24 24" fill="none" stroke="currentColor"'
' stroke-width="2" stroke-linecap="round">'
'<path d="M21 12a9 9 0 1 1-6.2-8.56"/></svg>' in strip
), "the house loading arc (decorative, aria-hidden) must ship"
assert '<span class="attach-preview-status-text">Uploading image…</span>' in strip
# exactly ONE status part, in the pinned order: img → name →
# status → remove (inside the strip, before its closing tag).
assert strip.count('class="attach-preview-status"') == 1
i_img = strip.find('class="attach-preview-img"')
i_name = strip.find('class="attach-preview-name"')
i_status = strip.find('class="attach-preview-status" hidden')
i_remove = strip.find('id="attach-remove"')
assert -1 < i_img < i_name < i_status < i_remove, (
"the status part sits after the name, before the remove button"
)
# the pre-phase parts are byte-untouched (the phase-123 contract).
assert '<img class="attach-preview-img" alt="">' in strip
assert 'aria-label="Remove the attached image"' in strip
def test_enter_exit_upload_state_are_the_exact_affordance_writers() -> None:
"""The two tiny helpers next to showAttachPreview / clearAttachedImage
(A5, owner-confirmed 2026-09-27 — the double-spam block is the
BOTH-halves affordance). enterUploadState: the strip gains the
.is-uploading class (task 02's CSS hooks it), the status part
unhides, the remove ✕ disables (the attachment is mid-flight), and
the Send button disables with the "Uploading…" label. exitUploadState
is the mirror image — and its idle restore is BYTE-IDENTICAL to
setUiState's idle label ("Send"), so the success path hands the
button to runTurn's setUiState("thinking") as usual and the
failure path leaves a fully editable composer. The attach family
is null-guarded (the house pattern); the send refs are
unconditional (the composer is required). setUiState / uiState /
the turn loader are NEVER touched — the upload is PRE-turn.
The module-scope ref is the strip's status part (the text is
static — no attachStatusText)."""
js = _app_js()
enter = _fn(js, "enterUploadState")
exit_ = _fn(js, "exitUploadState")
# entry — the full affordance, in the pinned order.
i_preview = enter.find('attachPreview.classList.add("is-uploading")')
i_status = enter.find("attachStatus.hidden = false")
i_remove = enter.find("attachRemove.disabled = true")
i_send = enter.find("sendBtn.disabled = true")
i_label = enter.find('sendLabel.textContent = "Uploading…"')
assert -1 < i_preview < i_status < i_remove < i_send < i_label
# exit — the mirror image, ending on the exact idle label.
i_preview = exit_.find('attachPreview.classList.remove("is-uploading")')
i_status = exit_.find("attachStatus.hidden = true")
i_remove = exit_.find("attachRemove.disabled = false")
i_send = exit_.find("sendBtn.disabled = false")
i_label = exit_.find('sendLabel.textContent = "Send"')
assert -1 < i_preview < i_status < i_remove < i_send < i_label
# the idle restore is byte-identical to setUiState's idle label —
# the same literal "Send", so no stale "Uploading…" can survive.
assert 'sendLabel.textContent = inFlight ? "Stop" : "Send";' in js
# the house null-guard pattern on the attach family only (a New
# chat mid-upload hides the strip; the class on a hidden element
# is harmless and still cleared at resolution); the send refs are
# unconditional (the chat view cannot exist without the composer).
for fn_src in (enter, exit_):
for guarded in ("attachPreview", "attachStatus", "attachRemove"):
assert f"if ({guarded})" in fn_src, f"{guarded} must be null-guarded"
assert "if (sendBtn)" not in fn_src and "if (sendLabel)" not in fn_src
# the PRE-turn contract: neither helper may touch the turn's
# state owners (the turn loader spins only when the turn starts).
for fn_src in (enter, exit_):
assert "setUiState" not in fn_src
assert "uiState" not in fn_src
assert "turnLoader" not in fn_src
assert "innerHTML" not in fn_src, "the house rule: textContent only"
# the module-scope ref: the status part (no text ref — static).
assert 'const attachStatus = document.querySelector(".attach-preview-status");' in js
assert "attachStatusText" not in js
def test_handle_send_wraps_the_await_with_the_upload_state() -> None:
"""In ``handleSend``'s attachedImage branch: ``enterUploadState()``
lands EXACTLY ONCE — after the ``attachUpload = true;`` guard line
and BEFORE ``await uploadAttachedImage`` (the moment the upload
starts — never before, never twice); ``exitUploadState()`` lands
EXACTLY ONCE — after the ``attachUpload = false;`` line (the
helper never throws, so this is reached on BOTH success and
failure) and BEFORE the ``if (path === null) return;`` block
(the failure path exits the state, then returns with the banner +
the editable composer + the attachment for the retry). The
``attachUpload`` guard is byte-untouched, the upload helper is
still called from exactly one site (the def + the call), and
``handleSend`` itself never calls ``setUiState`` (the pre-turn
contract — the turn's own state takes over inside runTurn)."""
js = _app_js()
# the EXACT handleSend body (brace matched — the file-order
# neighbors of handleSend carry their own setUiState references in
# their docstrings; the pin is on this function alone).
body = _fn(js, "handleSend")
i_guard = body.index("if (attachUpload) return;")
i_arm = body.index("attachUpload = true;")
i_enter = body.index("enterUploadState();")
i_await = body.index("await uploadAttachedImage(attachedImage.file)")
i_release = body.index("attachUpload = false;")
i_exit = body.index("exitUploadState();")
i_block = body.index("if (path === null) return;")
assert -1 < i_guard < i_arm < i_enter < i_await < i_release < i_exit < i_block, (
"guard → arm → ENTER → await → release → EXIT → block, in order"
)
assert body.count("enterUploadState();") == 1, "entered exactly once per upload"
assert body.count("exitUploadState();") == 1, (
"exited exactly once — the helper never throws, one line covers both paths"
)
# the pre-phase contract is byte-untouched: the guard, the single
# call site of the upload helper (def + call = 2 occurrences),
# and the A8 block line.
assert (
"if (attachUpload) return; // a second submit mid-upload: "
"the first owns the send (never two uploads, never two turns)" in js
)
assert js.count("uploadAttachedImage") == 2
# handleSend itself never drives the turn's UI state — the upload
# is PRE-turn (uiState stays idle across the whole flight); the
# call is absent (the word survives only in the phase-48 comment).
assert "setUiState(" not in body
#: The node harness (the test-document-viewer pattern): the REAL
#: extracted enterUploadState / exitUploadState run against stub
#: element refs; every transition is snapshotted as a JSON row —
#: [tag, isUploading, statusHidden, removeDisabled, sendDisabled,
#: label]. Plain strings only (the script is concatenation-assembled).
_UPLOAD_NODE_HEAD = """
const _classes = new Set();
const attachPreview = {
classList: {
add: (c) => _classes.add(c),
remove: (c) => _classes.delete(c),
contains: (c) => _classes.has(c),
},
};
const attachStatus = { hidden: true };
const attachRemove = { disabled: false };
const sendBtn = { disabled: false };
const sendLabel = { textContent: "Send" };
const show = (tag) => {
console.log(JSON.stringify([
tag,
_classes.has("is-uploading"),
attachStatus.hidden,
attachRemove.disabled,
sendBtn.disabled,
sendLabel.textContent,
]));
};
"""
_UPLOAD_NODE_TAIL = """
show("initial");
enterUploadState();
show("enter");
exitUploadState();
show("exit-success");
enterUploadState();
show("re-enter");
exitUploadState();
show("exit-failure");
"""
#: node availability (the test-document-viewer pattern).
HAVE_NODE = shutil.which("node") is not None
def _run_node(script: str) -> str:
proc = subprocess.run(["node", "-e", script], capture_output=True, text=True, timeout=60)
assert proc.returncode == 0, f"node failed: {proc.stderr}"
return proc.stdout
@pytest.mark.skipif(not HAVE_NODE, reason="node not available")
def test_upload_state_transitions_under_node() -> None:
"""Behavioral check (node) of the REAL enterUploadState /
exitUploadState against stub element refs (the test-document-
viewer pattern), across the pinned transition set:
* entry: the strip gains .is-uploading, the status part unhides,
the remove ✕ disables, the Send disables with "Uploading…";
* exit (success path): the mirror image — class removed, status
re-hidden, remove re-enabled, Send re-enabled with "Send";
* exit (failure path — the same call, banner the failure's
signal, not the strip's): byte-identical to the success exit —
the state is fully cleared either way (the user can retry or
remove).
"""
js = _app_js()
fns = "\n".join(_fn(js, name) for name in ("enterUploadState", "exitUploadState"))
rows = [
json.loads(line)
for line in _run_node(_UPLOAD_NODE_HEAD + fns + _UPLOAD_NODE_TAIL).splitlines()
if line
]
by_tag = {row[0]: row for row in rows}
# row = [tag, isUploading, statusHidden, removeDisabled, sendDisabled, label]
assert by_tag["initial"] == ["initial", False, True, False, False, "Send"]
assert by_tag["enter"] == ["enter", True, False, True, True, "Uploading…"], (
"entry is the full affordance — strip + both halves of the button"
)
# success and failure exits are byte-identical: fully cleared, the
# idle label restored exactly.
for tag in ("exit-success", "exit-failure"):
assert by_tag[tag] == [tag, False, True, False, False, "Send"], tag
# a re-entry (the retry after a failed upload) re-arms the full
# affordance — the helpers are idempotent in both directions.
assert by_tag["re-enter"] == ["re-enter", True, False, True, True, "Uploading…"]
# =====================================================================
# Phase 132 (task 02, TODO L8) — the strip's uploading CSS (the visual
# half of the affordance: the dimmed thumbnail, the hidden name slot,
# the spinning --ink-soft status, the dimmed remove ✕, the reduced-
# motion still, and the verified house :disabled language on the Send)
# =====================================================================
def _phase132_css() -> str:
"""The Phase 132 CSS block in styles.css — from its comment marker
to the next top-level rule (the ``.composer`` that follows the
``.attach-preview*`` family) — with the comments stripped, so the
raw-color audit sees only declarations."""
css = _css()
start = css.find("/* Phase 132 (task 02, TODO L8):")
assert start != -1, "the Phase 132 CSS block must exist in styles.css"
end = css.find("\n.composer {", start)
assert end != -1, "the Phase 132 block must sit before the .composer rule"
return re.sub(r"/\*.*?\*/", "", css[start:end], flags=re.S)
def test_the_uploading_strip_dims_the_thumbnail_and_keeps_the_name_slot() -> None:
"""The in-flight strip: the thumbnail DIMS (opacity 0.5 — it is
decorative, no contrast duty), and the filename HIDES with
visibility (NOT display/width — the flex: 1 slot keeps its measured
space, so the status takes the row's right hand without reflowing
the pinned 44px remove button; the strip height is stable)."""
css = _css()
m = re.search(r"\.attach-preview\.is-uploading \.attach-preview-img \{(.*?)\}", css, re.S)
assert m, "the uploading thumbnail must dim"
assert "opacity: 0.5;" in m.group(1)
m = re.search(r"\.attach-preview\.is-uploading \.attach-preview-name \{(.*?)\}", css, re.S)
assert m, "the uploading name must hide"
body = m.group(1)
assert "visibility: hidden;" in body
assert "display: none" not in body and "width: 0" not in body, (
"the name slot must keep its space (visibility, not removal)"
)
def test_the_uploading_status_row_is_the_ink_soft_small_text() -> None:
"""The status part's row: the house inline-flex language (the icon
+ the text as one unit), --ink-soft at the strip's small text size
(the name's 0.85rem), and min-width 0 (the row's clamp — at 320px
the text wraps rather than clipping; verified in the task's browser
pass: one line at 320px, no clip)."""
m = re.search(r"\.attach-preview-status \{(.*?)\}", _css(), re.S)
assert m, ".attach-preview-status must be styled"
body = m.group(1)
for needle in (
"display: inline-flex;",
"align-items: center;",
"gap: 0.35rem;",
"color: var(--ink-soft);",
"font-size: 0.85rem;",
"min-width: 0;",
):
assert needle in body, f".attach-preview-status must keep: {needle}"
def test_the_status_icon_is_the_14px_house_spin_at_the_sync_pace() -> None:
"""The arc: 14px (the strip's small-text scale), flex 0 0 auto (the
.sync-icon house pattern), animated with the HOUSE `spin`
keyframes at 1s linear — .sync-icon.is-spinning's in-flight pace
(the .spinner's 0.8s is the busy-turn arc's). The keyframes must
EXIST — a dangling animation-name would spin nothing and the
"in motion" state would be invisible."""
m = re.search(r"\.attach-preview-status-icon \{(.*?)\}", _css(), re.S)
assert m, ".attach-preview-status-icon must be styled"
body = m.group(1)
for needle in ("width: 14px;", "height: 14px;", "flex: 0 0 auto;"):
assert needle in body, f".attach-preview-status-icon must keep: {needle}"
assert "animation: spin 1s linear infinite;" in body
assert re.search(r"@keyframes spin \{\s*to \{ transform: rotate\(360deg\); \}\s*\}", _css()), (
"the house `spin` keyframes must exist (the arc's animation-name)"
)
def test_reduced_motion_stills_the_uploading_arc() -> None:
"""The house reduced-motion block: with prefers-reduced-motion the
arc STILLS (animation none) — the "Uploading image…" text + the
dimmed thumbnail then carry the state without motion (B5: never
motion alone; the state is text + dimming, not the spin)."""
m = re.search(
r"@media \(prefers-reduced-motion: reduce\) \{\s*"
r"\.attach-preview-status-icon \{ animation: none; \}\s*\}",
_css(),
)
assert m, "the house reduced-motion block must still the uploading arc"
def test_the_disabled_remove_x_dims_with_the_disabled_affordance() -> None:
"""The remove ✕ (task 01 disables it mid-flight via the attribute)
dims to 0.5 while uploading — the attribute is the state, the
opacity is its visual half (the same 0.5 as the dimmed thumbnail,
the house dim language)."""
m = re.search(r"\.attach-preview\.is-uploading \.attach-preview-remove \{(.*?)\}", _css(), re.S)
assert m, "the uploading remove button must dim"
assert "opacity: 0.5;" in m.group(1)
def test_the_disabled_send_rule_is_the_existing_house_rule_not_a_new_visual() -> None:
"""Task 02's verification step: the Send is disabled via the
`disabled` attribute (task 01) and the EXISTING house rule
``.send-btn:disabled`` already reads as disabled — the
--brand-busy background (the brand brightened 40% toward white,
visibly lighter than the enabled --brand fill, and a derived
variable that follows every saved theme) + the not-allowed
cursor. The pin says the rule is INTACT: the upload state must
not invent a second disabled look (no new color, no new weight).
(Verified in the task's browser pass: the rule applies to
#send-btn, which rides .send-btn.)"""
m = re.search(r"\.send-btn:disabled \{(.*?)\}", _css(), re.S)
assert m, "the house .send-btn:disabled rule must exist"
body = m.group(1)
assert "background: var(--brand-busy);" in body
assert "cursor: not-allowed;" in body
def test_the_phase132_css_ships_no_raw_color_and_holds_the_aa_pair() -> None:
"""The theme audit (the B4 byte-identical contract): every value
in the Phase 132 block is a theme variable or a neutral number —
NO raw hex/rgb/hsl (a custom theme repaints everything through the
variables). And the status text's --ink-soft on the strip's
--surface holds the AA floor: computed here against WCAG 2.1 from
the built-in :root literals (the theme phases' method) — 8.2:1,
≥ 4.5:1."""
block = _phase132_css()
assert not re.search(r"#[0-9a-fA-F]{3,8}\b", block), "no raw hex in the Phase 132 block"
assert not re.search(r"\b(?:rgb|rgba|hsl|hsla)\(|\bsrgb\b", block), (
"no raw rgb/hsl in the Phase 132 block"
)
ratio = _wcag_contrast(_root_color("--ink-soft"), _root_color("--surface"))
assert ratio >= 4.5, f"--ink-soft on --surface = {ratio:.2f}:1 (< 4.5:1 AA floor)"
def _root_color(name: str) -> str:
"""A built-in :root literal color (the theme phases audit the
built-in palette this way; a saved theme repaints through the
variables and is checked by the theme E2E)."""
m = re.search(rf"(?m)^\s*{name}:\s*(#[0-9a-fA-F]{{6}})", _css())
assert m, f"{name} must ship a plain built-in color in :root"
return m.group(1)
def _hex_rgb(value: str) -> tuple[int, int, int]:
h = value.lstrip("#")
return int(h[0:2], 16), int(h[2:4], 16), int(h[4:6], 16)
def _rel_luminance(rgb: tuple[int, int, int]) -> float:
"""WCAG 2.1 relative luminance (the house helper's math — the
test_responsive_polish / test_dark_tech_theme contrast helper)."""
def chan(c: int) -> float:
s = c / 255
return s / 12.92 if s <= 0.04045 else ((s + 0.055) / 1.055) ** 2.4
r, g, b = (chan(c) for c in rgb)
return 0.2126 * r + 0.7152 * g + 0.0722 * b
def _wcag_contrast(fg: str, bg: str) -> float:
l1, l2 = _rel_luminance(_hex_rgb(fg)), _rel_luminance(_hex_rgb(bg))
if l1 < l2:
l1, l2 = l2, l1
return (l1 + 0.05) / (l2 + 0.05)