fix(timetable): mechanical-layer stabilization — seeder Model A, Zod decimal drift, freeze-panes layout, ?day URL flicker #19
Assignees
bert.hausmans
Clear assignees
bert.hausmans
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bert.hausmans/crewli#19
Reference in New Issue
Block a user
Delete Branch "fix/timetable-stabilization"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Companion to
feat/timetable-session-4. After that PR landed, browser testing surfaced four mechanical-layer regressions that prevented the timetable from rendering. This PR fixes those four — and only those. UX parity with the prototype is explicitly out of scope and tracked asART-S4-UX-PARITY.What this fixes
Seeder Model A alignment (B1) —
ArtistTimetableDevSeederwas creating engagements at sub-event level; SCHEMA.md:1285 + RFC §10.2 (lines 1247-1257) + RFC §D17 specify festival-level engagements with per-sub-event performances. The controller filter was correct; the seeder was wrong. Aligned to Model A; 7 new feature tests lock the contract againstPerformanceController::index,ArtistEngagementController::index, andStageController::index.Zod decimal-as-string drift (B5) —
fee_amount,buma_percentage,vat_percentage,deposit_percentagewere declared asz.number().nullable(), but Laravel serialisesdecimal(N,M)columns as strings to preserve precision. First parse failure took the query to error state and surfaced "Kon timetable niet laden" in the UI. Fixed schemas + types + the one consumer that did arithmetic onfee_amount(PerformancePopover popover total). 5 new contract regression tests lock the wire format with three real-API fixtures (base / parked / multi-perf-engagement).Canvas freeze-panes layout (B2 + B3 + B4) — the pre-fix layout used a 2-cell grid that put ALL stage headers in one cell. With
block-size: 100%on the header, each header stretched to the entire canvas height (570px-tall single header for a 1-lane stage, hence "lanes are far too tall"). Restructured to one grid row per stage withposition: stickyfor the corner cell (z=3), TimeAxis row (z=2), StageHeader cells (z=2), and StageRow content (z=1). Single canvas-level scroll container; horizontal scroll syncs all panes.?dayURL flicker (B7) —isFlatEventwas derived fromsubEvents.length, returning a false-positivetruewhilesubEventswas still loading. This briefly seededvalidSubEventIdswith the festival id, whichuseActiveDay's corrective watcher used to rewrite the URL to?day={festival_id}(zero results), then immediately corrected to?day={subevent_id}once children loaded. Now derived fromeventDetail.event_type === 'event'— authoritative once the single-row endpoint resolves; no false-positive flat-event reading during the children-loading window.What this does NOT fix
This PR is mechanical layer only. UX parity with the prototype at
./resources/Crewli - Artist Timetable Management/is explicitly out of scope:All tracked as
ART-S4-UX-PARITYin BACKLOG.md, gated behind merge ofTEST-INFRA-001(Playwright Component Testing + visual regression) so the parity work lands with the right visual-test scaffolding.Lessons codified
Three diagnostic incidents in this branch (B1 controller-was-correct, B5 enum-shape-was-correct, UX divergence surfaced post-merge by browser test) led to formalising "audit before assume" as a CLAUDE.md principle — verify the canonical model against the artifact before writing a fix. Phase A of every fix prompt is now an explicit STOP-and-report gate. See the new "Diagnostic discipline: audit before assume" section in CLAUDE.md (commit
9c59f80).Test count delta
389 → 402 (+13: 7 backend feature tests for the seeder/controller contract, 4 row-height helper unit tests, 4 StageHeaderCell prop-seam tests, 5 schema contract regression tests covering 3 wire-shape variants + a number-fee_amount regression guard). All jsdom-based — visual UX parity is not validated by these and won't be until TEST-INFRA-001 + TEST-VISUAL-001 land.
Post-merge action required
Seeder semantics changed in B1 (festival-level engagements per Model A). Anyone with an existing local DB must run
php artisan migrate:fresh --seedafter pulling main; without this, local environments will show stale sub-event-level engagements that don't match the new controller filter expectations.Merge order
This PR follows
feat/timetable-session-4. Not standalone. Rebase on updated main after the Session 4 PR merges; conflicts should be minimal since this branch was cut fromfeat/timetable-session-4.🤖 Generated with Claude Code
Phase A diagnosed an empty SPA timetable as a controller filter bug. B1.1's schema-verify gate proved the opposite: the seeder violates Model A, the controllers are correct. Canonical model (Model A) per: - dev-docs/SCHEMA.md:1285 artist_engagements.event_id → festival OR flat event - dev-docs/SCHEMA.md:1329 performances.event_id → sub-event OR flat event ("show host") - dev-docs/RFC-TIMETABLE-Artist-Timetable-Module.md:1247-1257 (§10.2 contract) "performance.event_id must be flat event OR a sub-event of the engagement.event_id festival" - dev-docs/RFC-TIMETABLE-Artist-Timetable-Module.md:455-477 (§D17) "Friday + Saturday under one combined deal = 1 engagement, 2 performances" — only works if engagement is at festival level Controller audit (B1.2): all five filters in api/app/Http/Controllers/Api/V1/Artist/{PerformanceController, ArtistEngagementController, StageController}.php already match Model A. No controller changes needed. Seeder change (B1.3) — single consistent fix: ArtistTimetableDevSeeder::seedForFestival now creates one engagement per (artist, festival) instead of per (artist, sub-event). When the same artist recurs across iterations on different sub-events, the existing engagement is reused and another performance is added (the D17 multi-perf path). Performances continue to carry event_id = sub-event. Same model fix in seedForSeries (engagement at parent series, performance at week sub-event). seedForFlatEvent already conformed (engagement.event_id = performance.event_id = the flat event itself). Existence-check semantics shift from `where event_id = $subEvent->id` to `where event_id = $festival->id` (or $parent->id for series). Numerically the test counts hold because the bucket-cycling makes scheduled artists distinct within the festival window. Tests (B1.4) — new TimetableSeederControllerIntegrationTest with 7 assertions: - engagement.event_id is at festival level (DB invariant) - performance.event_id is at sub-event level (DB invariant) - GET /performances?day={subEvent} returns non-empty + correct event_ids - GET /performances unfiltered returns all sub-event performances - GET /performances?stage_id=null returns the seeded parked perf - GET /engagements returns engagements with event_id = festival - GET /stages returns 5 stages with event_id = festival This locks the visible-symptom regression from Session 4: an empty SPA timetable on a freshly-seeded festival cannot land again silently. Existing ArtistTimetableDevSeederTest (4 tests) and the broader Artist suite (121 tests) all stay green. composer analyse + Pint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Restructures the canvas so the spreadsheet-feel works correctly with the seeder's 14 stages: horizontal scroll moves the rows AND the TimeAxis together; vertical scroll moves the rows but keeps TimeAxis pinned; both panes intersect at a fixed corner cell. Diagonal trackpad scroll behaves naturally because there's only one scroll container. DOM restructure (E2 — sticky resolves to its nearest scroll ancestor; fixed by giving sticky elements the right scroll-container parent instead of patching with absolute positioning): .tt-page__canvas position: relative; overflow: auto └ .tt-page__layout display: grid; grid-template-columns: 200px auto; inline-size: max-content ├ .tt-page__corner sticky top:0 left:0 z=3 ├ .tt-page__axis sticky top:0 z=2 (full 1872px wide, no clip) └ for each stage: ├ .tt-page__header-cell sticky left:0 z=2 │ └ <StageHeaderCell :row-height-px="row.rowHeightPx"> └ .tt-page__row-cell normal z=1 (height = same value) └ <StageRow> Z-index ladder (E1) is documented in the page CSS: corner=3, axis row=2, header rail=2, row content=1, blocks=auto. Popover + AddPerformanceDialog stay above via Teleport-to-body. Drops the broken pre-stabilization layout: - `grid-template: "corner axis" 28px "stages rows" 1fr / 200px 1fr` that put ALL stage headers in ONE grid cell (cause of "lanes too tall" via headers stretching to 100% of the 570px cell) - nested `overflow: auto` on `.tt-page__rows` (cause of horizontal-scroll desync — only the rows pane scrolled, axis stayed put) - `overflow: hidden` on `.tt-page__axis` (E4 — clipped axis ticks beyond the 1fr cell width) - `<GridBg :total-height="0" />` which was a no-op anyway; gridlines now render directly on each `.tt-page__row-cell` background `inline-size: max-content` on the layout grid forces it wider than the canvas viewport, so `overflow: auto` on the canvas actually fires a horizontal scrollbar. Without this, the `auto` second column shrinks to viewport and nothing overflows. The page now passes `:row-height-px` to StageHeaderCell (B2 seam, now load-bearing). Both header and row cell get the same explicit blockSize inline so the freeze panes align pixel-for-pixel under whatever laneCount each stage resolves to. Visual scroll/alignment proof is deferred to TEST-VISUAL-001 — jsdom cannot verify position:sticky behavior, scrollbar visibility, or pixel alignment of the freeze panes. This is a known limitation, not a test gap. B4 covers the structural assertions jsdom CAN verify. All 389 existing tests still pass; production build smoke clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>B4 — jsdom-runnable assertions for the structural pieces of B2/B3. apps/app/tests/unit/lib/timetable/row-height.test.ts (4 tests): - laneCount=0 → 52px (Math.max(1, 0) fallback path) - laneCount=1 → 52px (single-lane stage row) - laneCount=3 → 148px - laneCount=10 → 484px (10 × 48 + 4) apps/app/tests/component/StageHeaderCell.test.ts (4 tests): - row-height-px prop applies as inline blockSize on the root - prop omitted → no inline blockSize set (legacy `block-size: 100%` CSS path takes over for any caller still relying on parent-driven sizing) - 484px for laneCount=10 round-trips through the prop without truncation - conflict badge renders only when conflictCount > 0 (existing behavior; locked in as part of touching this surface) Visual scroll/alignment proof (sticky-left freeze pane, sticky-top axis, horizontal scroll cohesion across 14 stages, diagonal trackpad scroll, pixel-perfect header↔row alignment) is deferred to TEST-VISUAL-001 explicitly: jsdom does not compute position:sticky offsets, scrollbar visibility, layout overflow chains, or scroll containment ancestry. This is a known limitation of jsdom-based component testing — not a test gap in this branch. The sticky behavior, z-index ladder, and DOM structure are all in place per E1-E4; their validation requires a real browser, which is exactly what the Playwright CT migration on TEST-INFRA-001 + TEST-VISUAL-001 unlocks. No existing tests asserted the old broken layout (no references to the deprecated `tt-page__rows`, `tt-page__stages`, or `<GridBg>` in tests/). The unused GridBg component file remains on disk; deleting it is a stylistic cleanup outside this stabilization scope. Test count: 389 → 397. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Phase A diagnosed the "Kon timetable niet laden" browser symptom as Zod schema drift. The prompt's hypothesis (enum {value, label} mismatch) was incorrect — the schema already uses the enumLabel() wrapper for every enum field. The actual drift is decimal-cast columns: Laravel serialises `decimal(N,M)` columns as strings to preserve precision, but the schema expected numbers, so the very first response triggered a ZodError. Affected fields, all on `artist_engagements`: fee_amount decimal(10,2) → wire `"11503.58"`, schema was z.number() buma_percentage decimal(5,2) → wire `"7.00"`, schema was z.number() vat_percentage decimal(5,2) → wire `"21.00"`, schema was z.number() deposit_percentage decimal(5,2) → wire `"…"`, schema was z.number() Backend has no explicit `decimal:N` cast on these columns (api/app/Models/ArtistEngagement.php:64-85 — the `casts()` method covers the enums + booleans + dates + integers, but skips decimals). Per the strategic decision (frontend adapts, backend stays): - schemas/timetable.ts: four fields → z.string().nullable() - types/timetable.ts: matching ArtistEngagement interface fields → `string | null` - PerformancePopover.vue:129: only consumer doing arithmetic on a decimal field; coerce at the use site via Number(...).toFixed(2). Single line. - tests/component/PerformanceBlock.test.ts + tests/a11y/axe.test.ts: spot-checked mocks; the two with hand-built engagement payloads flipped fee_amount/buma_percentage/vat_percentage from numbers to strings to match the new schema. No other mocks needed updating. The {value, label} enum wrapper claim in the prompt was specifically debunked in Phase A — every consumer (Wachtrij, PerformanceBlock, WachtrijCard, PerformancePopover, AddPerformanceDialog, page entry) already uses .value/.label access against an enumLabel-wrapped schema. B6 will lock the wire-format contract with a real-API fixture regression test. All 397 tests still pass; typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>apps/app/tests/unit/schemas/timetableContractShape.test.ts (NEW, 5 tests): - base shape: one performance with stage assigned + full engagement (Bert's browser-tested sample, field-for-field). Asserts decimal-as- string contract on fee_amount/buma_percentage/vat_percentage AND enum-label wrapper on booking_status AND nested computed object. - parked shape: stage_id=null, stage=null (Wachtrij case) - multi-perf shape: two performances sharing engagement_id (RFC §D17 "Friday + Saturday under one combined deal") - sanity: individual performanceSchema parses each fixture element - regression guard: a payload with NUMBER fee_amount throws (locks out the pre-B5 bug class) Every fixture spells out explicit `null` for the schema's nullable-but- required fields (timestamps, notes, deal_breakdown) so the nullable() vs optional() distinction is exercised, not glossed over. Schema surface change to support the test: apps/app/src/schemas/timetable.ts now EXPORTS performanceArraySchema (previously a private const inside useTimetable.ts). apps/app/src/composables/api/useTimetable.ts imports the shared one instead of redeclaring it locally — single source of truth for the array shape consumers and tests share. Test count: 397 → 402 (+5). Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Phase A finding A5 traced this race in the browser logs: GET .../performances?day={festival_id} → 200, 0 results ← wrong day GET .../children → 200, 3 sub_events GET .../performances?day={subevent_id} → 200, 13 results ← correct The pre-fix `isFlatEvent` was: computed(() => !subEvents.value || subEvents.value.length === 0) While `subEvents` was still loading (undefined), `!undefined` is `true`, so isFlatEvent erroneously returned `true` for festivals during the loading window. dayOptions then took the flat-event branch and seeded validSubEventIds with the FESTIVAL id. useActiveDay's corrective watcher rewrote the URL to `?day={festival_id}` and fired a wasted query that returned zero results (correct semantics — performances live at sub-event level — but waste + visible URL flicker). Fix: computed(() => eventDetail.value?.event_type === 'event') EventResource always serialises event_type (verified at api/app/Http/Resources/Api/V1/EventResource.php:26). EventTabsNav already consumes event_type / is_festival from the same shape (apps/app/src/components/events/EventTabsNav.vue:175,266) so this is the canonical signal, not a one-off addition. New behavior trace: - Both queries pending → eventDetail=undefined → isFlatEvent=false → festival branch returns (subEvents ?? []).map(...) → validSubEventIds=[] → activeDayId=null → usePerformances.enabled=false → NO fetch - subEvents resolves first → festival branch populates dayOptions → fetch fires with correct sub-event id - eventDetail resolves first to flat event → flat branch fires → fetch with eventDetail.id (correct) - eventDetail resolves first to festival → still false until subEvents → no false-positive flat-event fetch 402 tests still pass; typecheck + lint + production build all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>New section in CLAUDE.md after "Order of work for each new module". Three consecutive incidents in the timetable sprint led to formalising this principle: - B1 (controller assumed buggy, seeder was wrong) — Phase A's schema-verify gate against SCHEMA.md:1285 + RFC §10.2 inverted the fix direction. - B5 (enum-shape assumed drifted, decimals were wrong) — Phase A's field-by-field response audit caught the actual decimal-as-string drift before any "fix" against the wrong hypothesis was written. - Timetable UX (test-passing layer diverged from prototype) — the mechanical-vs-UX split surfaced via browser test, not via the 389-test suite which all agreed with the buggy state. Pattern across all three: the initial hypothesis was wrong. The fix prompts ALL gated Phase A as STOP-and-report; the schema/contract/ prototype audit was reviewed before any code was written. Codifying this as an explicit project principle so future fix prompts inherit the gate by default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Three trigger upgrades + one new entry, in priority order: TEST-INFRA-001 — trigger upgraded from "before opening Sessie 5" to "eerstvolgende sprint na merge van fix/timetable-stabilization", with explicit dependency: ART-S4-UX-PARITY and all Sessie 5+ work gate on TEST-INFRA-001 merge. Reden quote captures the three sprint-blok incidents that proved jsdom-tests do not protect against schema / filter / UX drift. TEST-VISUAL-001 — scope expanded to use the prototype HTML at `./resources/Crewli - Artist Timetable Management/` as the visual baseline source (not hand-curated screenshots). Added explicit state matrix per surface: PerformanceBlock 8 states + B2B + cascade-pulse; PerformancePopover full detail; AddPerformanceDialog drag-mode + button-mode; Wachtrij filtered/grouped axes. Trigger remains "tweede toevoeging na TEST-CONTRACT-001" inside the TEST-INFRA-001 sprint. TEST-CONTRACT-001 — unchanged. Trigger ("eerste e2e na TEST-INFRA-001 lands") was already correct. ART-S4-UX-PARITY (NEW) — captures Bert's screenshot-report findings as a seed list grouped A/B/C/D (component-shape / interaction / logic / AddPerformanceDialog two-mode). Explicit pointer at the bottom to the Phase A finalization report for the full 20-item itemisation with severity ratings. Trigger gates Sessie 5 + all subsequent Artist-domain frontend work behind ART-S4-UX-PARITY merge. Spelling consistency: VEE-001 entry "formalized" → "formalised" to match British-English already used elsewhere in the doc and now mandated by the new CLAUDE.md "Diagnostic discipline" section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>