[Badri-Rajendran]

Problem

PR #41 shipped the chat redesign with a known list of follow-ups:

1. Seal numbers changed after a reload. A streamed answer listed its sources in retrieval order, and a reload listed them by relevance. Tests missed it because test sessions expire objects on commit while production's SessionLocal doesn't (expire_on_commit=False).
2. Sending while scrolled up didn't scroll to the new question.
3. A connection lost mid-stream showed the browser's raw error (for example "network error"), and apiStream never released its reader.
4. The phone drawer didn't trap focus, although it claims aria-modal.
5. A finished answer wasn't announced. The streaming live region was removed at done.

It also left four minor review findings:

• no hide_parameters on the DB engine;
• the rename length checked before trimming, contrary to ADR 0027;
• ADR 0027's timeout bound omitted the CMS pricing call;
• a weak failed-commit test.

Implementation

One commit per fix, each with its CHANGELOG bullet.

Backend

• Seal order.
  • _save_answer sorts sources by relevance (highest first), then by chunk id and label.
  • Message.sources orders the same way in SQL, with COLLATE "C" so ties sort byte-wise, as in Python.
  • (chunk_id, source) is unique per answer, so the order is total.
  • No migration.
• hide_parameters=True on the engine: bound values stay out of SQL errors and logs.
• ThreadRenameRequest trims in a mode="before" validator, then applies min_length=1, max_length=200. Non-strings are left for pydantic to refuse.
• ADR 0027 gets a dated correction note in its Status section: each tool round can add an 8 s CMS call, so the theoretical bound passes Azure's 240 s. Its original text is kept, following how other ADRs record amendments.

Frontend

• apiStream wraps the reader loop in try/catch/finally. An abort is rethrown as is; any other failure becomes the existing "Can't reach PolicyPal…" ApiError. reader.cancel() always runs.
• Transcript
  • A new draft appearing (a question just sent) pins the view and scrolls, wherever the reader was.
  • Streaming text still follows only while pinned.
  • An always-mounted role="status" region says "Answer ready, N sources", counting documents as the seals do. It is empty while an answer streams, so identical texts still announce, and loaded history is silent.
• Focus trap
  • trapTab moves from SourcesPanel to features/chat/focus.js, where both chat users of it now share it.
  • The phone drawer traps Tab, and the chat behind it gets aria-hidden through a display: contents wrapper.
  • inert was tried and dropped: Playwright showed it blurred the opening button in the same commit, so focus couldn't return there on close.

Validation

• Backend: ruff clean, 737 passed, coverage 94.52% (≥85), alembic check clean, bandit clean, and pip-audit found no vulnerabilities.
• Frontend: eslint clean, 440 passed, and vite build passes.
• New tests fail without their fix:
  • the seal-order test, which uses a kept_on_commit fixture in the test file to run sessions as production does, leaving conftest.py unchanged;
  • the hidden-parameters test;
  • the mid-stream drop test.
• Playwright against the local stack:
  • seals 1–7 are identical live and after a reload;
  • a send from 390 px up brings the question into view;
  • going offline mid-answer shows "Can't reach PolicyPal…", and no stream is left busy;
  • the phone drawer keeps Tab (and Shift+Tab) inside, hides the chat, and returns focus to "Open your questions" on close;
  • "Answer ready, 7 sources" and then "Answer ready, 4 sources" were announced, and reloaded history was silent.

Security / DB / API impact

• Security: less exposure. SQL errors no longer carry bound values (emails, questions, answers). Postgres's own message can still quote a value it failed to parse.
• DB: no migration and no schema change. Only the ORDER BY used when loading an answer's sources changes.
• API: done and GET …/messages now list sources in the same order. PATCH /api/chat/threads/<id> accepts a title that fits after trimming, and still returns 422 for a blank, overlong or non-string title. There are no new endpoints.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F4R82wzcgXPBsgyeNRkPqW

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.