From 73ea2d7fed61fd54318ab98ca75260f6911c72a1 Mon Sep 17 00:00:00 2001 From: Kawa Date: Mon, 13 Apr 2026 10:22:32 +0200 Subject: [PATCH] docs(09): research phase UX tech debt closure Co-Authored-By: Claude Sonnet 4.6 --- .../09-ux-tech-debt-closure/09-RESEARCH.md | 573 ++++++++++++++++++ 1 file changed, 573 insertions(+) create mode 100644 .planning/phases/09-ux-tech-debt-closure/09-RESEARCH.md diff --git a/.planning/phases/09-ux-tech-debt-closure/09-RESEARCH.md b/.planning/phases/09-ux-tech-debt-closure/09-RESEARCH.md new file mode 100644 index 0000000..f4580b7 --- /dev/null +++ b/.planning/phases/09-ux-tech-debt-closure/09-RESEARCH.md @@ -0,0 +1,573 @@ +# Phase 9: UX Tech Debt Closure - Research + +**Researched:** 2026-04-13 +**Domain:** FastAPI / HTMX / Alpine.js / Playwright — frontend UX defect closure +**Confidence:** HIGH (all findings grounded in direct codebase inspection) + +--- + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions + +**Plan structure (3 plans, sequential):** +- 09-01: UX-01 + driver upload 500 fix (bundled). TDD: failing pytest first, fix root cause in `imptune/api/drivers.py`, then add inline driver upload to printer form template, HTMX OOB swap refreshes `` refreshed +- Auto-select newly uploaded driver (overrides any prior selection — user-confirmed behavior) +- TDD mandatory: failing test → red → fix → green → atomic commit +- Fix the specific 500 root cause only; do not add global exception swallowing + +**UX-02 specifics:** +- Playwright headless test in `tests/` (e2e subdir or similar) +- pytest-playwright plugin; app started via uvicorn subprocess/thread fixture +- chromium browser (default) +- Test file path + green pytest run = VALIDATION.md evidence + +**UX-03 specifics:** +- Three distinct GET routes (not `?kind=` param) +- Reuse generators from `imptune/services/` (same as package export) +- `Content-Type: text/plain; charset=utf-8`, `Content-Disposition: attachment; filename=".ps1"` +- New `

Scripts

` block in `printer_detail.html` above/below existing Export section +- Existing package export buttons stay untouched + +### Claude's Discretion +- Exact Alpine.js / HTMX OOB wiring syntax for the driver refresh +- Whether to inline the upload control inside the printer form or stack it above +- Playwright config file location and browser choice (chromium is default, fine) +- Exact wording of the 3 download link labels +- Whether the 3 new script routes live in `imptune/api/printers.py` or a new `imptune/api/scripts.py` +- Test fixture format for the 500 repro (real driver ZIP vs. synthetic ZIP) + +### Deferred Ideas (OUT OF SCOPE) +- Hardening `/drivers/upload` against unknown future exceptions (global try/except, structured error pages) +- A "Download all scripts as ZIP" button alongside the 3 individual links +- Playwright tests for anything beyond UX-02 +- Refactoring `upload_driver` into a service layer +- Fixing `/drivers/upload` to return JSON for programmatic clients + + +--- + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|-----------------| +| UX-01 | After a new driver is uploaded on the printer form, the DriverDesc dropdown refreshes automatically (no manual page reload) — verified live in browser | HTMX OOB swap pattern documented; existing `upload_driver` handler and printer form template inspected; OOB fragment wiring described | +| UX-02 | PRNT-03 Alpine.js IP→port auto-derivation is verified live in a real browser session, with the verification recorded in VALIDATION.md | Alpine.js `@input` handler already present in `printer_form.html:14`; Playwright + pytest-playwright integration pattern documented | +| UX-03 | The printer detail page exposes direct download links for each generated script (install / uninstall / detect) in addition to the package export buttons | Script routes ALREADY EXIST in `imptune/api/scripts.py` — only template links are missing; generator functions confirmed reusable | + + +--- + +## Summary + +Phase 9 closes three UX defects carried over from v1.0, plus one HTTP 500 bug blocking UX-01. All work is surgical: two of the three requirements touch existing files in limited ways, and one (UX-03) is nearly complete — the server-side routes already exist, only the template links are missing. + +**UX-01** requires the most work. The printer form has no inline upload control today; the upload endpoint works but throws HTTP 500 in at least one code path. The fix requires: (1) diagnosing and TDD-fixing the 500, (2) adding an inline `
` + file input to `printer_form.html`, (3) extending `upload_driver` to emit an HTMX OOB fragment that refreshes the driver ``. + +**UX-02** is a verification task only: the Alpine.js handler already exists and works (line 14 of `printer_form.html`). The requirement is to produce permanent machine-readable evidence by writing a Playwright headless test. No application code changes needed — only a new dev dependency and a new test file. + +**UX-03** is almost entirely done: `imptune/api/scripts.py` already implements the three GET routes (`/printers/{id}/scripts/install`, `.../uninstall`, `.../detect`) with correct `Content-Disposition` headers. The only gap is the URL shape (no `.ps1` extension in current routes) and the missing template links in `printer_detail.html`. Decision: whether to rename routes to include `.ps1` extension or keep as-is is a Claude's Discretion call — the `.ps1` extension in URLs was specified in CONTEXT.md decisions, so new routes or route aliases must match. + +**Primary recommendation:** Read `imptune/api/scripts.py` before implementing UX-03 — the routes exist, check if renaming vs. adding aliases is cleaner. For UX-01, reproduce the 500 in the test suite first before touching template code. + +--- + +## Standard Stack + +### Core (already in use — no new prod dependencies) + +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| FastAPI | 0.115.* | HTTP routing, response types | Project standard | +| HTMX | (baked into `/static`) | Partial HTML swaps, OOB swap | Project standard for dynamic UI | +| Alpine.js | (baked into `/static`) | Inline reactive state (`x-data`, `@input`) | Project standard for client-side reactivity | +| Jinja2 | 3.1.* | HTML template rendering | Project standard | +| Peewee | 3.17.* | ORM for Driver/Printer queries | Project standard | +| pytest + httpx | >=8.0 / >=0.27 | API integration tests | Project standard | + +### New Dev Dependency (UX-02 only) + +| Library | Version | Purpose | When to Use | +|---------|---------|---------|-------------| +| pytest-playwright | latest stable | Playwright integration for pytest | UX-02 headless browser test | +| playwright | latest stable | Browser automation (chromium) | UX-02 headless browser test | + +**Installation (dev only):** +```bash +pip install pytest-playwright playwright +playwright install chromium +``` + +Add to `requirements-dev.txt`: +``` +pytest>=8.0 +httpx>=0.27 +pytest-playwright +playwright +``` + +### What Already Exists (Do Not Rebuild) + +| Problem | Existing Solution | Location | +|---------|------------------|----------| +| Script download routes (install/uninstall/detect) | Already implemented | `imptune/api/scripts.py` | +| Script generators | `render_install`, `render_uninstall`, `render_detect` | `imptune/generators/script_generator.py` | +| Driver upload handler | `upload_driver` at `POST /drivers/upload` | `imptune/api/drivers.py` | +| Alpine.js IP→port derivation | `@input` handler on `ip_address` input | `printer_form.html:14` | +| HTMX partial response pattern | `_error_response()` + template response | `imptune/api/drivers.py`, `imptune/api/printers.py` | + +--- + +## Architecture Patterns + +### Pattern 1: HTMX OOB (Out-of-Band) Swap + +**What:** HTMX allows a single response to update multiple DOM regions. The primary content targets the element specified in `hx-target`; additional `hx-swap-oob="true"` fragments in the same response are swapped into their respective `id`-matched DOM elements. + +**When to use:** When a single user action (driver upload) needs to update two independent regions (the driver list on the current view AND the driver `` currently has no `id` — it must be given one (e.g., `id="printer-form-driver-select"`). + +**Response structure required:** +```html + +
+ ... existing driver list content ... +
+ + + +``` + +**How the handler knows it's called from the printer form:** Check for `HX-Target` header value, or include a hidden sentinel field in the inline upload form (e.g., ``). The sentinel field is simpler and more explicit. + +**Source:** HTMX documentation — `hx-swap-oob` attribute (HIGH confidence, direct HTMX docs concept, also in CONTEXT.md decisions) + +### Pattern 2: Inline Upload Form Within an Existing Form + +**What:** The printer form is a single `` element. The inline driver upload must NOT be nested inside that form (invalid HTML). It must be a separate `` element, visually grouped near the driver ` + ... + + + + + + + + +
+``` + +**Anti-pattern:** Nesting `
` inside `` — browsers silently ignore the inner form; the upload will never fire. + +### Pattern 3: Playwright Test with App Fixture + +**What:** pytest-playwright provides `page` and `browser` fixtures. The app must be running and accessible on a URL before the test can load pages. Use a `pytest` fixture that starts uvicorn in a background thread. + +**Fixture pattern:** +```python +# tests/e2e/conftest.py +import threading +import time +import pytest +import uvicorn +from imptune.main import app + +@pytest.fixture(scope="session") +def live_server(tmp_path_factory): + """Start the FastAPI app on a random port for E2E tests.""" + # Setup tmp data dir (similar to unit test conftest) + ... + config = uvicorn.Config(app, host="127.0.0.1", port=8765, log_level="error") + server = uvicorn.Server(config) + thread = threading.Thread(target=server.run, daemon=True) + thread.start() + time.sleep(0.5) # Let server start + yield "http://127.0.0.1:8765" + server.should_exit = True +``` + +**UX-02 test body:** +```python +def test_port_autofill(page, live_server): + page.goto(f"{live_server}/printers") + # Trigger printer form (HTMX-loaded partial or direct URL) + page.fill("input[name='ip_address']", "192.168.1.100") + # Alpine.js reacts synchronously on @input + expected = "IP_192_168_1_100" + assert page.input_value("input[name='port_name']") == expected +``` + +**Source:** pytest-playwright documentation (MEDIUM confidence — verified pattern from official docs concept; exact fixture API confirmed via library knowledge) + +### Pattern 4: Script Download Routes (UX-03 — Near Zero Work) + +**Existing state:** `imptune/api/scripts.py` already provides: +- `GET /printers/{printer_id}/scripts/install` → `Content-Disposition: attachment; filename="install.ps1"` +- `GET /printers/{printer_id}/scripts/uninstall` → `Content-Disposition: attachment; filename="uninstall.ps1"` +- `GET /printers/{printer_id}/scripts/detect` → `Content-Disposition: attachment; filename="detect.ps1"` + +**Gap vs. CONTEXT.md decision:** CONTEXT.md locked URLs include `.ps1` extension in the path (e.g., `/printers/{id}/scripts/install.ps1`). Current routes do NOT have `.ps1` in the path — they use `/scripts/install` without extension. + +**Resolution (Claude's Discretion):** Two options: +1. Add new routes with `.ps1` extension alongside existing routes (minimal risk, no breakage) +2. Rename existing routes (simpler, but technically a breaking change if anything already links to the old URLs — unlikely since there are no template links yet) + +Option 1 (add aliases) is safer. The existing routes have no template links so breakage risk is zero either way, but aliases are unambiguous. + +**Template gap:** `printer_detail.html` has NO links to any script download routes. The entire `

Scripts

` block needs to be added. + +### Anti-Patterns to Avoid + +- **Nesting forms:** Never put the inline driver upload `` inside the printer `` — browsers silently reject nested forms. +- **Using `hx-swap-oob` without a matching DOM id:** The OOB target element must exist in the current page DOM with the exact matching `id`. +- **Returning OOB fragment on the `/drivers` page route:** The `/drivers` page does NOT contain `#printer-form-driver-select`. The upload handler must detect its caller context and only emit the OOB fragment when called from the printer form. +- **Installing Playwright in the production image:** Playwright headless browsers are large. Keep in `requirements-dev.txt` only. The Docker production image must not install Playwright. +- **Running Playwright tests in the standard unit test suite without a live server:** pytest-playwright tests require a running HTTP server — they cannot use `TestClient`. Use a session-scoped live server fixture. + +--- + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| OOB DOM updates after upload | Custom JS to manually refresh the `` fragment even when called from the `/drivers` standalone page. The fragment is silently discarded by HTMX (no matching DOM id) but the presence of junk HTML in the response may cause unexpected behavior. + +**How to avoid:** Gate the OOB fragment emission on the sentinel field: `if request.form.get("caller") == "printer_form": emit_oob = True`. Only include the OOB block in that branch. + +### Pitfall 3: Alpine.js `x-data` Scope and the Inline Upload Form + +**What goes wrong:** The inline upload form is placed inside the printer form's Alpine `x-data` div. If upload state (e.g., `uploading: false`) needs to be tracked, it must be added to the `x-data` initialization object on the outer div — not declared in a nested Alpine component, which would create a separate reactive scope that can't interact with the parent form's `ip`/`port` variables. + +**How to avoid:** Extend the existing `x-data="{ ip: ..., port: ..., portEdited: ... }"` declaration with upload state: `x-data="{ ip: ..., port: ..., portEdited: ..., uploading: false }"`. + +### Pitfall 4: Playwright Test Startup Race + +**What goes wrong:** The live server fixture starts uvicorn in a thread and immediately yields — the server may not be bound and listening before the first test navigates to a URL, causing connection refused. + +**How to avoid:** Add a brief readiness poll after starting the server thread (e.g., retry `GET /health` up to 10 times with 100ms sleep). The existing `GET /health` endpoint is available for this purpose. + +### Pitfall 5: pytest-playwright Not Finding Chromium Binaries + +**What goes wrong:** `playwright install` must be run separately from `pip install playwright`. A `pip install` alone does not download browser binaries. + +**How to avoid:** Document `playwright install chromium` as a required setup step in the plan. In CI/CD this would be a setup step; for local dev, the implementing developer must run it once. + +### Pitfall 6: UX-03 — Confusing Existing Routes with CONTEXT.md URL Shape + +**What goes wrong:** Developer reads `imptune/api/scripts.py`, sees the three routes exist, declares UX-03 done, forgets (a) the `.ps1` extension in the URL was locked in CONTEXT.md, and (b) there are no template links yet. + +**How to avoid:** Check both the route URL shape and the template. Two tasks: add URL aliases/renames if needed, and add template links. + +--- + +## Code Examples + +### OOB Swap in Upload Handler + +```python +# imptune/api/drivers.py — extended upload_driver response branch +# Source: HTMX OOB swap pattern + existing project codebase + +from fastapi.responses import HTMLResponse + +@router.post("/upload", response_class=HTMLResponse) +def upload_driver(request: Request, file: UploadFile) -> HTMLResponse: + # ... existing validation and persistence logic ... + + # Build driver_data for template + drivers = list(Driver.select().order_by(Driver.uploaded_at.desc())) + driver_data = [] + for d in drivers: + names = json.loads(d.driver_desc) if d.driver_desc else [] + driver_data.append({"driver": d, "names": names}) + + # Detect caller context via form field + # (caller field included in inline upload form in printer_form.html) + called_from_printer_form = False + try: + form_data = ... # access via request if needed, or pass as Form() param + called_from_printer_form = (form_data.get("caller") == "printer_form") + except Exception: + pass + + if called_from_printer_form: + # Return primary fragment + OOB select fragment + return templates.TemplateResponse( + request=request, + name="partials/driver_upload_with_oob.html", + context={ + "driver_data": driver_data, + "new_driver_id": new_driver.id, + "parsed": parsed, + }, + ) + else: + # Existing behavior — driver list only + return templates.TemplateResponse( + request=request, + name="partials/driver_list.html", + context={"driver_data": driver_data, "parsed": parsed}, + ) +``` + +### Adding Sentinel Field to Inline Upload Form + +```html + + + + + + Uploading… +
+``` + +### Adding `id` to the Existing Driver Select + +```html + + +``` + +### OOB Partial Template Fragment + +```html + + + +{% include "partials/driver_list.html" %} + + + +``` + +### UX-03 Template Links + +```html + +

Scripts

+ + Download Install Script + + + Download Uninstall Script + + + Download Detect Script + +``` + +### TDD Pattern for 500 Repro (Follow Existing Test Structure) + +```python +# tests/test_driver_upload.py — add regression test FIRST, before fixing + +def test_upload_500_regression(client: TestClient) -> None: + """POST /drivers/upload must not return 500 for a valid driver ZIP. + + This test was added to capture the repro of the HTTP 500 reported + during Phase 8 kickoff (2026-04-13). It should go RED first, then + GREEN after the fix is applied. + """ + zip_bytes = _make_driver_zip() # or use real driver ZIP that triggers the bug + resp = client.post( + "/drivers/upload", + files={"file": ("driver.zip", zip_bytes, "application/zip")}, + ) + assert resp.status_code != 500, f"Upload returned 500: {resp.text}" + assert resp.status_code == 200 +``` + +--- + +## State of the Art + +| Old Approach | Current Approach | Impact on Phase 9 | +|--------------|------------------|-------------------| +| Script routes did not exist | `imptune/api/scripts.py` ships in v1.0 with all three routes | UX-03 is mostly done — only template links missing | +| No per-script download links in template | Template has Export section but no script links | Add `

Scripts

` block to `printer_detail.html` | +| Alpine.js port handler untested | Handler exists at `printer_form.html:14`, known-working | UX-02 only needs a Playwright test as evidence | +| No inline driver upload in printer form | v1.0 had separate `/drivers` upload flow | UX-01 requires both the 500 fix and the inline upload addition | + +**Key discovery — UX-03 near-complete:** The three script download routes already exist in `imptune/api/scripts.py`, registered in `imptune/main.py`, with correct `Content-Disposition` headers. The only work is: +1. Determine if URL shape needs `.ps1` extension (CONTEXT.md says yes — `/printers/{id}/scripts/install.ps1`) +2. Add template links in `printer_detail.html` + +Current route URLs (`/scripts/install`) differ from locked decision URLs (`/scripts/install.ps1`). Claude's Discretion on whether to add aliases or rename. + +--- + +## Open Questions + +1. **What triggers the HTTP 500 in `POST /drivers/upload`?** + - What we know: Reported during Phase 8 kickoff (2026-04-13); exact traceback not captured in planning docs + - What's unclear: Whether it's `parse_inf()`, `DriverStore.save()`, or `Driver.get_or_create()` failing + - Recommendation: The TDD plan (reproduce → diagnose → fix) is correct; do not guess the fix from code reading. The most likely candidates based on code inspection: (a) `parse_inf()` with unusual INF content, (b) `DriverStore.save()` with a missing directory in certain startup sequences. + +2. **Passing `caller` field from an HTMX form — Form() parameter or body access?** + - What we know: FastAPI `Form()` parameters work for `application/x-www-form-urlencoded` and `multipart/form-data` + - What's unclear: The upload endpoint uses `UploadFile` which is already multipart; adding `caller: str = Form("")` as a parameter alongside `file: UploadFile` should work with FastAPI's multipart handling + - Recommendation: Add `caller: str = Form("")` parameter to `upload_driver` signature — FastAPI handles mixed multipart fields + files natively. + +3. **Playwright live server fixture — thread vs. subprocess?** + - What we know: uvicorn can run in a thread via `uvicorn.Server.run()`; subprocess is more isolated but harder to share DB state + - What's unclear: Whether the thread-based approach handles the Peewee SQLite connection properly in a test context (SQLite has per-thread connection behavior) + - Recommendation: Use thread-based fixture but initialize a fresh in-memory or tmp SQLite DB for the E2E session, same pattern as `conftest.py` `tmp_data_dir` fixture. Alternatively, set up one driver + printer record before starting the server so the form has data to interact with. + +--- + +## Validation Architecture + +> `workflow.nyquist_validation` is `true` in `.planning/config.json` — this section is required. + +### Test Framework + +| Property | Value | +|----------|-------| +| Framework | pytest >= 8.0 | +| Config file | None — no pytest.ini or pyproject.toml detected | +| Quick run command | `pytest tests/ -x -q` | +| Full suite command | `pytest tests/ -v` | +| E2E run command | `pytest tests/e2e/ -v` (after Playwright install) | + +### Phase Requirements → Test Map + +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| UX-01 (500 fix) | `POST /drivers/upload` never returns HTTP 500 for valid input | unit/integration | `pytest tests/test_driver_upload.py::test_upload_500_regression -x` | ❌ Wave 0 — add test case | +| UX-01 (OOB swap) | Upload response includes OOB `