Skip to content

Commit 37a6f97

Browse files
committed
fix(manage): give catalog browser root spacer-safe min-height
The CatalogBrowser page root used a fixed h-full box with no overflow handling, so overflowing catalog content would spill past it and hide the assistant-launcher clearance spacer, like the four page wrappers fixed in 533a657. Switch it to min-h-full shrink-0 the same way. Also records the consolidated agy review outcome in the plan: rate limiter finding rejected as stale (prune already exists), index and activities page roots deliberately left h-full (their nested scroll design depends on the percentage-height chain, and the spacer compression is the intended launcher clearance there).
1 parent 1022edd commit 37a6f97

2 files changed

Lines changed: 3 additions & 2 deletions

File tree

apps/frontend-manage/src/components/catalog/CatalogBrowser.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ function CatalogBrowser({
3737
}
3838

3939
return (
40-
<div className="h-full">
40+
<div className="min-h-full shrink-0">
4141
<PendingSharingRequests />
4242
<ObjectImport
4343
collectionName={metaData?.getCatalogCollectionInfo?.name}

project/2026-07-23-pr-5109-assistant-production-readiness-plan.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,5 @@ Architecture note before scaling tool count: lecturer READ tools talk directly t
236236

237237
- 2026-07-24: **Future-work P1-P8 executed + review-thread batch.** Roland approved working the roadmap with Opus subagents, explicitly excluding P9 i18n (separate chat design workstream — also parks the de.ts "Assistant" CTA review thread 3342859759). Six-agent fleet over disjoint file clusters, every diff personally reviewed before commit: `6107df65f` (build: `@klicker-uzh/prisma#build` now declares `src/prisma/client/**` as turbo outputs — CI `check-types` failed on cache-restored builds missing the generated client; proven locally via mv-away + cache-restore, confirmed green in CI), `bb3d8f714` (P1 telemetry toggle `CHAT_ENABLE_AI_TELEMETRY`; P2 in-memory sliding-window rate limiters, 30/5min chat + 10/5min confirm, 429+Retry-After, per-pod caveat documented; P4 replay protection — mcp-lecturer signs a `randomUUID` `jti`, confirm claims it once via in-memory TTL map, reuse → 409, no-jti tokens pass for rollout compat; OPENAI_API_KEY fail-fast; `APP_ORIGIN_API` throws in production instead of localhost fallback; `@/src/...` aliases — CodeRabbit's literal `@/services/...` suggestions were wrong for the tsconfig), `c087d2e11` (server-side `sanitizeAsPath` via zod transform + Template/Instance/Quiz ID prompt lines + manage-side `MAX_ASPATH_LENGTH` clamp parity + stale-context clear on failed sanitization), `aa700873e` (dialog keyboard-modal: focus-into-panel, Tab trap, aria-modal; new-tab link uses `embed:false` URL), `384c1ad3d` (P8 dismiss action with terminal collapsed state + pure `applyDismiss`/`isDismissible`; thread inline-margin cleanup; `ThreadWelcomeCapabilities` extracted = thermo #2), `527a34fcd` (P3: tampered-token 403, confirm 401, mid-stream error, malformed envelope E2E), `2a569dbba` (pwa chatbot error redirects locale-prefixed), `80cca1c8b` (P6 dev CSP: `ALLOWED_FRAME_ANCESTORS=https://*.localhost` — host wildcard covers namespaced worktree origins). Rebutted with evidence (no change): mcp-lecturer Dockerfile pnpm pattern (13/13 repo convention), workflow hardening (0/32 convention; repo-wide task chip spawned), CourseChatDrawer dist/ops import (459-file convention), formatToolName + query-key cap (already addressed). Task chips: missing frontend-pwa `/error` page; `packages/util` jwt `expiresIn: 0` bug.
238238
- 2026-07-24: **Launcher pointer-interception regression fixed (shards 2/4/7).** CI on `6107df65f`: check-types green, `Y-manage-assistant` passed in CI for the first time — but the now-compiled floating launcher broke three other specs, each `<button data-cy="manage-assistant-open" class="... fixed ... z-30 ..."> intercepts pointer events` on an end-of-page bottom-right control (`groupActivity-save-submission-grading`, `live-quiz-template-submit`, `submit-create-answer-collection`) — a real UX bug for lecturers, not a test artifact. Fix `533a657f8` took three verified iterations driven by an agent-browser rect-intersection harness against this worktree's own stack: (1) container `pb-20 md:pb-24` → ZERO effect — browsers do not extend a scroll container's scrollable overflow area by its own end padding (every overlap case measured exactly `viewport − 61px` footer, padding swallowed); (2) in-flow end-spacer in Layout → still clipped on pages whose root wrapper is `h-full w-full`: the fixed 100%-height box with visible overflow positions following flow content under the overflowing region; (3) final: Layout end-spacer (`h-20 md:h-24`, gated on the assistant flag) + page-root wrappers (AnswerCollections/Chatbots/MediaLibrary/UserGroupsManagement) switched `h-full` → `min-h-full shrink-0` — `shrink-0` is load-bearing: without it the overflowing column-flex container compresses a `min-h-full` item straight back to 100% (explicit min-height discards the content-based automatic minimum). Final measurements: 93px clearance in both prior worst cases (1280×800 16-entry form, 1280×577), wrapper `offsetHeight === scrollHeight` exactly; cancel-click works; course list unaffected. Ops: this worktree's auth app (3010) was crashed since stack boot — respawned in-container by cloning the live turbo process env from `/proc/<pid>/environ` (keeps the workspace-namespaced `APP_ORIGIN_AUTH`/`NEXTAUTH_URL`); delegated login verified after.
239-
- 2026-07-24: **P5+P7 handshake simplification + stream-error UI (`53e1473e9`).** Ack protocol (`klicker:manage-context-ack`, `nextMessageIdRef`/`ackedMessageIdRef`) and the `[300,1000,2500]`ms retry burst removed on BOTH sides — the ready→resend handshake plus send-on-load and send-on-change is the whole protocol now (resolves the deferred thermo #1 with the E2E safety net it demanded). Message-type literals centralized in `packages/types/src/manageAssistant.ts` (thermo #4; `@klicker-uzh/types` already a dependency of both apps, zero lockfile churn). thread.tsx wires `MessagePrimitive.Error` → muted inline `AssistantMessageError` note (`data-cy="chat-assistant-message-error"`, raw SDK error text deliberately not shown) — closes the silent-stream-failure product gap found during P3; subagent verified against installed `@assistant-ui/react@0.12.10` + `@assistant-ui/core` source AND an empirical throwaway vitest run of the real `Chat`+`DefaultChatTransport` against the exact mock SSE bodies: mid-stream error attaches `{type:'incomplete', reason:'error'}` to the partial message, malformed envelope synthesizes an empty assistant message via `createErrorAssistantMessage` — one wiring covers both. Y-manage-assistant now 13 tests: + dismiss flow (confirm-request spy asserts zero calls), + slow-hydration (script-fetch delay pushes hydration past the parent's load-time send; course-dashboard suggestions still arrive via handshake alone), + strengthened error asserts (both error tests assert the visible note; corrected a factually wrong pre-existing `toHaveCount(0)` assertion for the malformed case). Verified: chat check + 140/140, manage check + 3/3 assert tests, types build/check, playwright tsc + `--list` 13. Next: consolidated agy review of `64da087f5..53e1473e9`, push, thread replies (22 of 23; de.ts parked), PR body refresh, CI watch.
239+
- 2026-07-24: **P5+P7 handshake simplification + stream-error UI (`53e1473e9`).** Ack protocol (`klicker:manage-context-ack`, `nextMessageIdRef`/`ackedMessageIdRef`) and the `[300,1000,2500]`ms retry burst removed on BOTH sides — the ready→resend handshake plus send-on-load and send-on-change is the whole protocol now (resolves the deferred thermo #1 with the E2E safety net it demanded). Message-type literals centralized in `packages/types/src/manageAssistant.ts` (thermo #4; `@klicker-uzh/types` already a dependency of both apps, zero lockfile churn). thread.tsx wires `MessagePrimitive.Error` → muted inline `AssistantMessageError` note (`data-cy="chat-assistant-message-error"`, raw SDK error text deliberately not shown) — closes the silent-stream-failure product gap found during P3; subagent verified against installed `@assistant-ui/react@0.12.10` + `@assistant-ui/core` source AND an empirical throwaway vitest run of the real `Chat`+`DefaultChatTransport` against the exact mock SSE bodies: mid-stream error attaches `{type:'incomplete', reason:'error'}` to the partial message, malformed envelope synthesizes an empty assistant message via `createErrorAssistantMessage` — one wiring covers both. Y-manage-assistant now 13 tests: + dismiss flow (confirm-request spy asserts zero calls), + slow-hydration (script-fetch delay pushes hydration past the parent's load-time send; course-dashboard suggestions still arrive via handshake alone), + strengthened error asserts (both error tests assert the visible note; corrected a factually wrong pre-existing `toHaveCount(0)` assertion for the malformed case). Verified: chat check + 140/140, manage check + 3/3 assert tests, types build/check, playwright tsc + `--list` 13. Next: consolidated agy review of `64da087f5..53e1473e9`, push, thread replies (22 of 23; de.ts parked), PR body refresh, CI watch.
240+
- 2026-07-24: **Consolidated agy review integrated (`64da087f5..53e1473e9`).** Verdict approve-with-nits, 2 findings. F1 (rate-limiter Map never deletes stale keys) REJECTED as stale: agy quoted a `createRateLimiter` closure that does not exist in this worktree — the real `SlidingWindowRateLimiter` prunes empty keys every 200 checks (`prune()` deletes zero-hit keys); its file links even pointed at the main checkout. F2 (remaining `h-full` page roots squashed by the Layout spacer) PARTIALLY ACCEPTED: `pages/index.tsx:307` and `activities.tsx:233` REJECTED — their roots anchor a deliberate nested-scroll design (`h-full overflow-y-auto` root → inner `h-full overflow-y-auto` list scroller with static toolbar/filters); `min-h-full shrink-0` would break the percentage-height chain and make the whole page scroll, and the spacer's ~93px compression there IS the launcher clearance (the internal scroller now ends above the FAB, keeping last-row right-side actions clickable — browser-verified no interception on `/`). `CatalogBrowser.tsx:40` ACCEPTED — plain `h-full` with no overflow handling, same clipping class as the four wrappers fixed in `533a657f8`; children have no percentage-height reliance; switched to `min-h-full shrink-0`. Closing-question answers: Redis migration for limiter/jti stays a documented scale-out follow-up (per-pod is adequate at current deployment size); iframe focus placement is handled by the widget's open-effect focus-into-panel. manage tsc green. Next: push, thread replies, PR body, CI watch.

0 commit comments

Comments
 (0)