Repository navigation
feat(web): add a flock scope selector to the Dashboard, fixing the strip's scale fallback - #918
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Evidence at head SHA Before ( After (this branch, After, After, the picker open with a search in progress. After, a single flock selected — the picker closes on commit (one of the three runtime-found defects fixed in this PR) and the card rescopes. After, the only-one-accessible-flock state: All frames 1280×800 and 390×844, light and dark, device scale factor 1, viewport only.
|
…dings Fidelity round on PR #918 (owner directive: the mockup is the spec, #901). The Lay rate card's flock scope now matches docs/designs/916-production-scale/production-flock-selector-v2.html in DOM order and control shape rather than approximating it: - One full-width selector (eyebrow "Flock" + value + chevron) replaces the separate All-flocks chip beside an MUI Autocomplete. It opens a picker dialog built to the mockup's own shape -- a 44px close button, a labelled search input filtering the already-loaded flock list client-side, and an "All flocks" choice pinned ABOVE the scrolling result list, never a row inside it. - A context caption ("{N} accessible flocks / the flock's name * range") sits under the selector. - The scale caption now reads "Eggs per day * complete-day scale" / "* partial days only" / "* no recorded figures", and Peak/Avg always render a sentence ("Peak --", "No complete-day average") instead of hiding when null. - DayStrip gains the mockup's three-item legend (Complete/Partial/No entry). - The hen-day KPI moves from the top of the card to the bottom, after the strip, on both desktop and phone. Also folds in three findings from Codex's review of c5d60d4: - The picker's only reset path (choosing "All flocks" inside the dialog) sets scope and the displayed value from the SAME state now, so there is no second "committed" mirror left to desync (finding 1). - The page-level "everything failed" decision now waits for the production report's own first outcome too, not just the four panel reads, so a farm where only those four fail no longer hides an already-loaded Lay rate card behind the full-page error; `loading` itself still resolves as soon as the four settle, so a slow production fetch never blocks the page (finding 2). - A new test drives an explicit stale-response race (older scope resolves after a newer one) and asserts the newer figures survive; verified by deleting the trend effect's `cancelled` guard and confirming the test goes red, then reverting (finding 3). Web suite: 3148/3148 passing, typecheck clean. Playwright, rebuilt isolated stack: the flock-scope spec's 6 tests (5 desktop + 1 @phone) and the full 63-test quick smoke suite (1 pre-existing skip) all green.
|
Fidelity round + Codex review findings, both done at head Mockup fidelityRebuilt the Lay rate card to match
Codex review of c5d60d4 — all three folded inFinding 1 (P2, picker Clear with no handler). The new single-selector design has exactly one reset path — choosing "All flocks" inside the dialog — and it sets Finding 2 (P2, "everything failed" excludes production). Finding 3 (P3, no test for the stale-response race). New test — confirmed locally, then reverted; the suite is green again with the guard back in place. Verification
Screenshots (all 20 "after" frames recaptured against this round's build; 1280×800 and 390×844, light and dark, device scale factor 1, viewport only) are attached in the next comment. The isolated |
|
All 20 frames, recaptured against head default-farm, All flocks — single selector, context caption, "partial days only" scale, legend, KPI at the bottom. readme-farm, All flocks — complete-day scale. Picker open — "Choose flock" header with 44px close, search filtered to "Sim House", "All flocks" pinned above the two matching results. Single flock selected — "Sim House A" scoped, complete-day scale, Complete-day avg + Peak. Only one accessible flock (
|
… generation, add picker keyboard nav and a flock-list-unavailable state (#918) Codex review, round 3 (c5d60d4..f6169b8): the dialog's search only filtered the first 500 already-loaded flocks, so a flock past that page was unreachable on a large farm — search is now server-paged and debounced (50 rows/250ms), matching the shared picker engine. The "everything failed" gate kept its first-ever panel/trend outcome forever, so an earlier failure could outlive a later refresh that succeeded (a role change flipping canSeeSales) — both outcomes are now keyed by a load generation counter. The results list gained Arrow/Home/End/Enter roving focus, and a failed flock-list read now shows the existing panel-error pattern with Retry instead of reading as an empty farm.
|
Round 3 pushed at Codex astra's four findings against c5d60d4..f6169b8:
Declining one finding, with evidence — not implemented: "a production request that never settles leaves the card loading indefinitely; no deadline." Verification: Screenshots below: the picker-open state at 1280/390, light/dark, plus one search-result frame — the only surfaces round 3 touched.
|
…eneration, and rerun production on a role change (#918) Codex review, round 4 (f6169b8..HEAD): the picker's own state (search, paging, keyboard nav) moves into a new FlockPickerDialog component with its own tests, so Dashboard.tsx only holds whether the dialog is open and the scope a pick produced. Within it: extension requests and the debounced search are now tied to a query generation, dropping a stale or closed-dialog response instead of corrupting the results/cursor; a failed discovery request shows its own unavailable state with Retry instead of reading as a real empty result; and keyboard roving focus skips a disabled Load more button. In Dashboard.tsx: the production-report effect now also reruns on a role change alone (canSeeSales), which the panels effect's own generation bump already did — without it, a role change with the flock set unchanged left no trend outcome recorded for the new generation, silently swallowing a genuine total failure. Retry on a failed flock-list read no longer clears the unavailable state before the retried read settles.
|
Round 4 pushed at Given how much state the picker dialog was carrying (query generation, load generation, paging cursor, dialog focus), the dialog now lives in its own component — Codex astra's six findings against f6169b8..bbc134e:
The two round-3 picker-mechanics tests that duplicated what now lives in FlockPickerDialog.test.tsx ( Verification: |
…re against a stale query or a closed dialog (#918) Codex review, round 5 (bbc134e..bbaf1ef): the round-4 extraction had narrowed the keyboard test down to the disabled-Load-more boundary case alone, dropping the Arrow/Home sweep, the search-to-results ArrowDown handoff, and Escape's return-focus-to-trigger behavior — all restored in one test against a real trigger button. The generation-race test covered a replacement search only; a Load More extension left pending across a query change, or across the dialog closing and reopening, was unguarded. Both gaps are test-only: the product code already had the line-104 generation guard from round 4.
|
Round 5 (last round) pushed at
Verification: |
Review loop — closed at
|
| Round | Head | What it fixed | Product defects |
|---|---|---|---|
| 1 | c5d60d4 |
TDD scale fix, server-side flock scoping, selector, three-locale Help; initial review | 2 (Clear left the card on the old flock; page-level error hid a loaded Lay Rate card) |
| 2 | f6169b8 |
Card rebuilt to the mockup's DOM order (single selector → context caption → scale figures → strip → legend → hen-day last); picker dialog with pinned "All flocks"; stale-response race test | 4 (search capped at the first 500 loaded flocks; stale failure across the farm-day rollover; no keyboard navigation; failed flock list shown as an empty farm) |
| 3 | bbc134e |
Server-paged debounced search; failure outcomes keyed by load generation; roving focus; unavailable state with Retry | 3 (old load-more page appended to newer results; role change advanced the generation without rerunning production; failed search shown as "No matching flocks") |
| 4 | bbaf1ef |
Picker extracted to FlockPickerDialog with its own tests; query-generation guard on all three request kinds and on close; Retry keeps the loading state; keyboard skips disabled buttons; fake-timer debounce and out-of-order generation tests |
0 |
| 5 | 31e4cce |
Test-only: pending load-more across query change and close/reopen; Arrow/Home, search-to-results and Escape-returns-focus assertions; all mutation-proven | 0 |
Declined with evidence (round 3): a production request that never settles leaves the card loading; web/src/api/client.ts raw() (line 151) applies no deadline to any read, so a timeout only here would be inconsistent. Carried to the owner's issue batch.
Verification at 31e4cce: typecheck clean, Vitest 3,159 across 133 files; at bbaf1ef the quick Playwright suite plus the new 6-test spec were green on a rebuilt isolated stack; .NET at c5d60d4 (server code unchanged since): Domain 491, Application 532 (ledger, adapter-reach and coupling-matrix guards), Integration 1,854 under Docker. CI green at every head through bbaf1ef; 31e4cce (tests only) is being watched. Frames: 24 at c5d60d4, 20 recaptured at f6169b8, picker/search frames at bbc134e.
Loop stopped deliberately after round 5: rounds 4 and 5 confirmed no product defect. Left to #914/#915 per SELECTION.md: range control (must not reset the flock choice), historical flock eligibility and saved-preference persistence.
Deslop pass —
|
f618a82 to
26ebcc5
Compare
…dings Fidelity round on PR #918 (owner directive: the mockup is the spec, #901). The Lay rate card's flock scope now matches docs/designs/916-production-scale/production-flock-selector-v2.html in DOM order and control shape rather than approximating it: - One full-width selector (eyebrow "Flock" + value + chevron) replaces the separate All-flocks chip beside an MUI Autocomplete. It opens a picker dialog built to the mockup's own shape -- a 44px close button, a labelled search input filtering the already-loaded flock list client-side, and an "All flocks" choice pinned ABOVE the scrolling result list, never a row inside it. - A context caption ("{N} accessible flocks / the flock's name * range") sits under the selector. - The scale caption now reads "Eggs per day * complete-day scale" / "* partial days only" / "* no recorded figures", and Peak/Avg always render a sentence ("Peak --", "No complete-day average") instead of hiding when null. - DayStrip gains the mockup's three-item legend (Complete/Partial/No entry). - The hen-day KPI moves from the top of the card to the bottom, after the strip, on both desktop and phone. Also folds in three findings from Codex's review of c5d60d4: - The picker's only reset path (choosing "All flocks" inside the dialog) sets scope and the displayed value from the SAME state now, so there is no second "committed" mirror left to desync (finding 1). - The page-level "everything failed" decision now waits for the production report's own first outcome too, not just the four panel reads, so a farm where only those four fail no longer hides an already-loaded Lay rate card behind the full-page error; `loading` itself still resolves as soon as the four settle, so a slow production fetch never blocks the page (finding 2). - A new test drives an explicit stale-response race (older scope resolves after a newer one) and asserts the newer figures survive; verified by deleting the trend effect's `cancelled` guard and confirming the test goes red, then reverting (finding 3). Web suite: 3148/3148 passing, typecheck clean. Playwright, rebuilt isolated stack: the flock-scope spec's 6 tests (5 desktop + 1 @phone) and the full 63-test quick smoke suite (1 pre-existing skip) all green.
… generation, add picker keyboard nav and a flock-list-unavailable state (#918) Codex review, round 3 (c5d60d4..f6169b8): the dialog's search only filtered the first 500 already-loaded flocks, so a flock past that page was unreachable on a large farm — search is now server-paged and debounced (50 rows/250ms), matching the shared picker engine. The "everything failed" gate kept its first-ever panel/trend outcome forever, so an earlier failure could outlive a later refresh that succeeded (a role change flipping canSeeSales) — both outcomes are now keyed by a load generation counter. The results list gained Arrow/Home/End/Enter roving focus, and a failed flock-list read now shows the existing panel-error pattern with Retry instead of reading as an empty farm.
…eneration, and rerun production on a role change (#918) Codex review, round 4 (f6169b8..HEAD): the picker's own state (search, paging, keyboard nav) moves into a new FlockPickerDialog component with its own tests, so Dashboard.tsx only holds whether the dialog is open and the scope a pick produced. Within it: extension requests and the debounced search are now tied to a query generation, dropping a stale or closed-dialog response instead of corrupting the results/cursor; a failed discovery request shows its own unavailable state with Retry instead of reading as a real empty result; and keyboard roving focus skips a disabled Load more button. In Dashboard.tsx: the production-report effect now also reruns on a role change alone (canSeeSales), which the panels effect's own generation bump already did — without it, a role change with the flock set unchanged left no trend outcome recorded for the new generation, silently swallowing a genuine total failure. Retry on a failed flock-list read no longer clears the unavailable state before the retried read settles.
…re against a stale query or a closed dialog (#918) Codex review, round 5 (bbc134e..bbaf1ef): the round-4 extraction had narrowed the keyboard test down to the disabled-Load-more boundary case alone, dropping the Arrow/Home sweep, the search-to-results ArrowDown handoff, and Escape's return-focus-to-trigger behavior — all restored in one test against a real trigger button. The generation-race test covered a replacement search only; a Load More extension left pending across a query change, or across the dialog closing and reopening, was unguarded. Both gaps are test-only: the product code already had the line-104 generation guard from round 4.
26ebcc5 to
8c6f95f
Compare
…dings Fidelity round on PR #918 (owner directive: the mockup is the spec, #901). The Lay rate card's flock scope now matches docs/designs/916-production-scale/production-flock-selector-v2.html in DOM order and control shape rather than approximating it: - One full-width selector (eyebrow "Flock" + value + chevron) replaces the separate All-flocks chip beside an MUI Autocomplete. It opens a picker dialog built to the mockup's own shape -- a 44px close button, a labelled search input filtering the already-loaded flock list client-side, and an "All flocks" choice pinned ABOVE the scrolling result list, never a row inside it. - A context caption ("{N} accessible flocks / the flock's name * range") sits under the selector. - The scale caption now reads "Eggs per day * complete-day scale" / "* partial days only" / "* no recorded figures", and Peak/Avg always render a sentence ("Peak --", "No complete-day average") instead of hiding when null. - DayStrip gains the mockup's three-item legend (Complete/Partial/No entry). - The hen-day KPI moves from the top of the card to the bottom, after the strip, on both desktop and phone. Also folds in three findings from Codex's review of c5d60d4: - The picker's only reset path (choosing "All flocks" inside the dialog) sets scope and the displayed value from the SAME state now, so there is no second "committed" mirror left to desync (finding 1). - The page-level "everything failed" decision now waits for the production report's own first outcome too, not just the four panel reads, so a farm where only those four fail no longer hides an already-loaded Lay rate card behind the full-page error; `loading` itself still resolves as soon as the four settle, so a slow production fetch never blocks the page (finding 2). - A new test drives an explicit stale-response race (older scope resolves after a newer one) and asserts the newer figures survive; verified by deleting the trend effect's `cancelled` guard and confirming the test goes red, then reverting (finding 3). Web suite: 3148/3148 passing, typecheck clean. Playwright, rebuilt isolated stack: the flock-scope spec's 6 tests (5 desktop + 1 @phone) and the full 63-test quick smoke suite (1 pre-existing skip) all green.
… generation, add picker keyboard nav and a flock-list-unavailable state (#918) Codex review, round 3 (c5d60d4..f6169b8): the dialog's search only filtered the first 500 already-loaded flocks, so a flock past that page was unreachable on a large farm — search is now server-paged and debounced (50 rows/250ms), matching the shared picker engine. The "everything failed" gate kept its first-ever panel/trend outcome forever, so an earlier failure could outlive a later refresh that succeeded (a role change flipping canSeeSales) — both outcomes are now keyed by a load generation counter. The results list gained Arrow/Home/End/Enter roving focus, and a failed flock-list read now shows the existing panel-error pattern with Retry instead of reading as an empty farm.
…eneration, and rerun production on a role change (#918) Codex review, round 4 (f6169b8..HEAD): the picker's own state (search, paging, keyboard nav) moves into a new FlockPickerDialog component with its own tests, so Dashboard.tsx only holds whether the dialog is open and the scope a pick produced. Within it: extension requests and the debounced search are now tied to a query generation, dropping a stale or closed-dialog response instead of corrupting the results/cursor; a failed discovery request shows its own unavailable state with Retry instead of reading as a real empty result; and keyboard roving focus skips a disabled Load more button. In Dashboard.tsx: the production-report effect now also reruns on a role change alone (canSeeSales), which the panels effect's own generation bump already did — without it, a role change with the flock set unchanged left no trend outcome recorded for the new generation, silently swallowing a genuine total failure. Retry on a failed flock-list read no longer clears the unavailable state before the retried read settles.
…re against a stale query or a closed dialog (#918) Codex review, round 5 (bbc134e..bbaf1ef): the round-4 extraction had narrowed the keyboard test down to the disabled-Load-more boundary case alone, dropping the Arrow/Home sweep, the search-to-results ArrowDown handoff, and Escape's return-focus-to-trigger behavior — all restored in one test against a real trigger button. The generation-race test covered a replacement search only; a Load More extension left pending across a query change, or across the dialog closing and reopening, was unguarded. Both gaps are test-only: the product code already had the line-104 generation guard from round 4.
…918) Deslop follow-up: measuring comment density against main caught the overall increase but missed block LENGTH — several branch-added blocks still ran 5-9 lines. Sixteen such blocks across Dashboard.tsx, FlockPickerDialog.tsx, the Playwright spec and lib/dashboard.ts are now at or under 4 lines, with the load-bearing invariants (the total-failure verdict spanning both effects and keyed to loadGenRef; why the picker's results/cursor clear in the same tick as a query change) kept intact. lib/dashboard.ts's pre-existing half (from main) is untouched; only the branch-added tail was trimmed.
…a stale retry verdict, cancel superseded reports, and be honest about a truncated flock count (#918) First proper review seat on #918, found three P2s and two P3s: - P2-1: the Morning collection panel's "Yesterday by close" caption derived from the SAME scoped trend the Lay rate card reads, so picking a flock changed a different panel's figure. It is now its own always farm-wide, single-day fetch, decoupled from scope. - P2-2: a successful flock-list Retry never updated the page-level failure verdict, so a stale "all four panels failed" record could survive a recovered flock list and hide the whole dashboard once the auto-triggered sole-flock production read then failed. The generation-ref machinery is replaced with two outcome states (pending/someOk/allFailed, pending/success/failure) that Retry now updates directly. - P2-3: a superseded production request was only ignored, never aborted, so it could sit in flight holding a report-concurrency permit. The trend effect now uses an AbortController per dispatch, threaded through apiGet/getProductionReport. - P3-4: the picker dialog focused its close button on open instead of the search box, and the search box was under the 44px touch-target floor. Fixed with autoFocus and a min-height override on the input wrapper. - P3-5: the accessible-flock count presented as exact past listFlocks's 500-row cap. It now reads "500+" once truncated. Every fix carries a test that fails on the pre-fix code and a local mutation check confirming it; see the PR reply for the exact evidence.
… CI flake in a picker test the branch didn't introduce (#918) A dashboard load fired three /reports/production requests (two adjacent trend windows plus the new yesterday-close fetch), three of the account's four shared report-concurrency permits (RateLimitingOptions.ReportsConcurrency: PermitLimit 4, QueueLimit 0, no queue — anything past the cap 429s rather than waiting). Two workers on the same farm opening the app together could exceed it. The two adjacent trend windows (current week, previous week) are now one daysBefore(today,14)..daysBefore(today,1) request, split client-side by the new splitProductionReport. Every report-level total is a plain sum over days[], computed the identical way server-side (ReportQueries.GetProductionAsync), so summing a subset of an already-fetched period reproduces exactly what a second request for that subset would have returned — verified both by a fast unit test against a hand-built fixture and, more importantly, against the real simulation server's own combined-vs-separate responses. A test pins the request count at two per load so the ceiling cannot creep back. Also fixes an unrelated CI flake in NamedEntityPicker.test.tsx exposed by this branch's CI run: a raw KeyboardEvent dispatch plus an empty act() raced MUI Autocomplete's controlled-state commit. Converted to userEvent + waitFor; two other raw-dispatch tests in that file were checked and are not the same race (one asserts a stable negative on a disabled control, the other needs the raw event's defaultPrevented, which userEvent does not expose), so they were left alone.
|
Two things folded into this push at Trend-window fold, per the concurrency review. A dashboard load fired three The two adjacent trend windows (current week, previous week) are now one
CI flake in I checked the rest of that file for the same shape (raw Verification: |
…dEntityPicker/FlockPicker (#918) Assessment found no reason FlockPickerDialog needed to exist: the mockup's own fidelity requirement — "All flocks" pinned above the results — is already solvable through the picker's existing slots.paper mechanism (the same one Load More renders through), and the original brief said to reuse the picker in the first place. Dashboard now opens the shared Dialog with FlockPicker inside it (the same nesting ExpensesPage already uses), and a new optional `pinnedChoice` prop threads fixed content above the listbox through NamedEntityPicker's engine — omitted by every other caller, so Daily entry/Feed/Water/Expenses/History are unaffected. The engine's search field also gets the 44px minHeight fix proven on the bespoke dialog, this time applied once for all five callers.
|
At Which row covers this. Why this matters more than the diff. Three defects from the last two rounds — the query-generation race on load-more, the missing initial focus on the search field, and the 37px tap target — all landed in mechanics Open owner decision, deliberately not resolved here. The mockup's own hand-rolled picker moves Home/End as list navigation (jump to first/last choice); Verification: full local suite ( |
The shared Dialog unmounts its children on close, so every reopen of the Lay-rate flock picker was a fresh engine mount, and Dashboard never told it what scope was already committed — every reopen showed aria-selected="false" on the previously picked flock and aria-pressed="false" on "All flocks", with no indicator of the active scope at all. Fixed by threading `scope` through FlockPicker's existing `controlledCommitted`/`controlledGeneration` props, plus `onClear` (wired to the same reset "All flocks" performs, since seeding a committed entity also surfaces the engine's own footer Clear link, previously dead code with nothing scoped). Checked empirically rather than assumed: this does pre-fill the reopened search field with the committed name, matching every other FlockPicker/CustomerPicker caller's own reopen behaviour, and does not narrow the results, since the seed only touches display text, never the discovery filter. Also corrects the "five callers" count from the prior round's comments to the real total: eleven pre-existing render sites (nine FlockPicker, two CustomerPicker) across seven screens.
|
Fixed at The fix. On the query-seeding constraint. Checked against the DOM rather than assumed: seeding Regression tests, both states, each mutation-verified (
Same four scenarios re-verified against a rebuilt, real-browser Corrected the caller count per your note: "all five callers" → eleven pre-existing render sites (nine Scope discipline held: Home/End untouched, CI: |
…omplete dayStrip() fell straight to a null peak whenever no day in the window was fully recorded, so height() drew every bar at the 2% floor regardless of the real totals behind them -- the exact symptom from #916 (a farm where 2 of 102 houses file shows every bar as a hairline stub despite a real 738-egg day). Add a "partial" fallback: when no complete day exists but at least one day has a recorded figure, scale against the largest of those instead. The complete-day-only average is untouched. A new `scale` field on DayStripData ("complete" | "partial" | "none") lets the caption say which pool is in use.
GET /api/v1/reports/production takes an optional flockId. The query narrows every scan in ReportQueries.GetProductionAsync (eggs, grade totals, and the hen-day exposure walk) together -- narrowing eggs without narrowing exposure would rate a healthy flock at a fraction of its real lay, the #780 defect in miniature. The endpoint checks IFlockRepository.GetByIdAsync first and returns 404 for a flock the caller cannot see, reusing FlockEndpoints' own existence-check pattern (the structural AccountId+flock-scope query filter is what makes that check double as the access check). Adds the FlockManagement reach the module ledger and coupling matrix now require, and three integration tests: scoped-vs-farm-wide partition, a flock outside the caller's own flock scope, and a flock from another tenant or naming nothing at all.
…916) Multiple accessible flocks: an All flocks toggle beside the existing NamedEntityPicker/FlockPicker (MUI Autocomplete, #898), All flocks kept outside the picker's own scrolling results per the approved mockup. Exactly one accessible flock: its name as plain text, no picker. The whole card -- bars, completeness, the complete-day average, and both hen-day comparison periods -- follows the chosen scope; every other Dashboard panel is unaffected, and the report fetch for the two hen-day windows is now a separate effect so a scope change never re-fetches them. With exactly one accessible flock, the sole flock's id feeds the SAME getProductionReport(from, to, flockId) call a manual pick would produce -- structural parity between the "only one" and "selected single" views, per SELECTION.md. Docs: a GLOSSARY.md entry for the strip's scale fallback and for the flock scope, an in-app glossary entry in en/es/tl, and an addition to the existing Dashboard help text.
… the aria mismatch (#916) Three defects found by end-to-end browser evidence against the isolated simulation stack, none caught by the unit suite: - The All flocks button was 36px tall, under the repo's 44px tap-target floor; phone.spec.ts's standing guard (every visible link/button in `main`) went red against it. - The scope picker's onCommit never closed the search, unlike every other FlockPicker caller in the app (Daily entry, Feed, Water, Expenses, History) -- confirmed live, not assumed from a prior code read. - trendLabel's accessible sentence branched on `max === null`, which the scale-rule fix already made true for BOTH "no data" and "partial fallback" -- so a screen reader was told "no peak or average" over a window whose visible caption and Peak figure showed a real number. Rewritten to branch on `scale` directly; trendStripLabelNoComplete is now provably unreachable and removed, replaced by trendStripLabelPartialScale. Adds tools/simulation/ui/specs/dashboard-flock-scope.spec.ts: the fixture's own farm-wide partial-day fallback, server-side rescoping through the picker, the one-flock plain-text state (restrictedWorker(), whose reads turn out to be flock-scoped too -- #613's structural filter, not the write-only behavior src/cast.ts's older comment describes), the only-one/selected-single report parity across two principals, and a @phone-tagged layout check. Verified against a rebuilt isolated stack: 5/5 new tests, 62/63 full smoke suite (1 pre-existing skip), the 44px guard, and tsc --noEmit all green.
Drop a no-op onSnapshot handler FlockPicker already defaults on its own, and fix a stale comment in dayStrip's height() that still said "the complete-day peak" after the fallback made that only sometimes true.
…dings Fidelity round on PR #918 (owner directive: the mockup is the spec, #901). The Lay rate card's flock scope now matches docs/designs/916-production-scale/production-flock-selector-v2.html in DOM order and control shape rather than approximating it: - One full-width selector (eyebrow "Flock" + value + chevron) replaces the separate All-flocks chip beside an MUI Autocomplete. It opens a picker dialog built to the mockup's own shape -- a 44px close button, a labelled search input filtering the already-loaded flock list client-side, and an "All flocks" choice pinned ABOVE the scrolling result list, never a row inside it. - A context caption ("{N} accessible flocks / the flock's name * range") sits under the selector. - The scale caption now reads "Eggs per day * complete-day scale" / "* partial days only" / "* no recorded figures", and Peak/Avg always render a sentence ("Peak --", "No complete-day average") instead of hiding when null. - DayStrip gains the mockup's three-item legend (Complete/Partial/No entry). - The hen-day KPI moves from the top of the card to the bottom, after the strip, on both desktop and phone. Also folds in three findings from Codex's review of c5d60d4: - The picker's only reset path (choosing "All flocks" inside the dialog) sets scope and the displayed value from the SAME state now, so there is no second "committed" mirror left to desync (finding 1). - The page-level "everything failed" decision now waits for the production report's own first outcome too, not just the four panel reads, so a farm where only those four fail no longer hides an already-loaded Lay rate card behind the full-page error; `loading` itself still resolves as soon as the four settle, so a slow production fetch never blocks the page (finding 2). - A new test drives an explicit stale-response race (older scope resolves after a newer one) and asserts the newer figures survive; verified by deleting the trend effect's `cancelled` guard and confirming the test goes red, then reverting (finding 3). Web suite: 3148/3148 passing, typecheck clean. Playwright, rebuilt isolated stack: the flock-scope spec's 6 tests (5 desktop + 1 @phone) and the full 63-test quick smoke suite (1 pre-existing skip) all green.
… generation, add picker keyboard nav and a flock-list-unavailable state (#918) Codex review, round 3 (c5d60d4..f6169b8): the dialog's search only filtered the first 500 already-loaded flocks, so a flock past that page was unreachable on a large farm — search is now server-paged and debounced (50 rows/250ms), matching the shared picker engine. The "everything failed" gate kept its first-ever panel/trend outcome forever, so an earlier failure could outlive a later refresh that succeeded (a role change flipping canSeeSales) — both outcomes are now keyed by a load generation counter. The results list gained Arrow/Home/End/Enter roving focus, and a failed flock-list read now shows the existing panel-error pattern with Retry instead of reading as an empty farm.
…eneration, and rerun production on a role change (#918) Codex review, round 4 (f6169b8..HEAD): the picker's own state (search, paging, keyboard nav) moves into a new FlockPickerDialog component with its own tests, so Dashboard.tsx only holds whether the dialog is open and the scope a pick produced. Within it: extension requests and the debounced search are now tied to a query generation, dropping a stale or closed-dialog response instead of corrupting the results/cursor; a failed discovery request shows its own unavailable state with Retry instead of reading as a real empty result; and keyboard roving focus skips a disabled Load more button. In Dashboard.tsx: the production-report effect now also reruns on a role change alone (canSeeSales), which the panels effect's own generation bump already did — without it, a role change with the flock set unchanged left no trend outcome recorded for the new generation, silently swallowing a genuine total failure. Retry on a failed flock-list read no longer clears the unavailable state before the retried read settles.
…re against a stale query or a closed dialog (#918) Codex review, round 5 (bbc134e..bbaf1ef): the round-4 extraction had narrowed the keyboard test down to the disabled-Load-more boundary case alone, dropping the Arrow/Home sweep, the search-to-results ArrowDown handoff, and Escape's return-focus-to-trigger behavior — all restored in one test against a real trigger button. The generation-race test covered a replacement search only; a Load More extension left pending across a query change, or across the dialog closing and reopening, was unguarded. Both gaps are test-only: the product code already had the line-104 generation guard from round 4.
Trim review-round narration from the production code: comments re-litigating what an earlier draft of this same unmerged branch did wrong are archaeology a future reader cannot act on, while the invariants they surround are kept. Dashboard.tsx was 27% comment lines against main's 12%; it is now 20%, on a file that grew by a subsystem. Also folds the duplicated `soleFlock`/`soleFlockId` expressions into one derivation. No behaviour change: typecheck clean, 3159 Vitest tests pass.
The first pass measured density and stopped there, leaving 18 branch-added comment blocks of 5+ lines. Cuts the spec header from 25 lines to 12 and removes the review-round narration the i18n catalog still carried.
…918) Deslop follow-up: measuring comment density against main caught the overall increase but missed block LENGTH — several branch-added blocks still ran 5-9 lines. Sixteen such blocks across Dashboard.tsx, FlockPickerDialog.tsx, the Playwright spec and lib/dashboard.ts are now at or under 4 lines, with the load-bearing invariants (the total-failure verdict spanning both effects and keyed to loadGenRef; why the picker's results/cursor clear in the same tick as a query change) kept intact. lib/dashboard.ts's pre-existing half (from main) is untouched; only the branch-added tail was trimmed.
…t-side The picker's search became a server-paged `listFlocks` call in the round-3 fix; this comment still described the pre-round-3 client-side filter, which is the opposite of what #916's API half exists to guarantee.
Three statements of the 2%-floor rationale in dashboard.ts collapse to one, `max` and `scale` stop pointing at each other in a circle, the picker-catalog note stops being made twice, and the new file drops its filename banner — only 5 of 104 non-test files carry one, so it is not a convention.
…a stale retry verdict, cancel superseded reports, and be honest about a truncated flock count (#918) First proper review seat on #918, found three P2s and two P3s: - P2-1: the Morning collection panel's "Yesterday by close" caption derived from the SAME scoped trend the Lay rate card reads, so picking a flock changed a different panel's figure. It is now its own always farm-wide, single-day fetch, decoupled from scope. - P2-2: a successful flock-list Retry never updated the page-level failure verdict, so a stale "all four panels failed" record could survive a recovered flock list and hide the whole dashboard once the auto-triggered sole-flock production read then failed. The generation-ref machinery is replaced with two outcome states (pending/someOk/allFailed, pending/success/failure) that Retry now updates directly. - P2-3: a superseded production request was only ignored, never aborted, so it could sit in flight holding a report-concurrency permit. The trend effect now uses an AbortController per dispatch, threaded through apiGet/getProductionReport. - P3-4: the picker dialog focused its close button on open instead of the search box, and the search box was under the 44px touch-target floor. Fixed with autoFocus and a min-height override on the input wrapper. - P3-5: the accessible-flock count presented as exact past listFlocks's 500-row cap. It now reads "500+" once truncated. Every fix carries a test that fails on the pre-fix code and a local mutation check confirming it; see the PR reply for the exact evidence.
… CI flake in a picker test the branch didn't introduce (#918) A dashboard load fired three /reports/production requests (two adjacent trend windows plus the new yesterday-close fetch), three of the account's four shared report-concurrency permits (RateLimitingOptions.ReportsConcurrency: PermitLimit 4, QueueLimit 0, no queue — anything past the cap 429s rather than waiting). Two workers on the same farm opening the app together could exceed it. The two adjacent trend windows (current week, previous week) are now one daysBefore(today,14)..daysBefore(today,1) request, split client-side by the new splitProductionReport. Every report-level total is a plain sum over days[], computed the identical way server-side (ReportQueries.GetProductionAsync), so summing a subset of an already-fetched period reproduces exactly what a second request for that subset would have returned — verified both by a fast unit test against a hand-built fixture and, more importantly, against the real simulation server's own combined-vs-separate responses. A test pins the request count at two per load so the ceiling cannot creep back. Also fixes an unrelated CI flake in NamedEntityPicker.test.tsx exposed by this branch's CI run: a raw KeyboardEvent dispatch plus an empty act() raced MUI Autocomplete's controlled-state commit. Converted to userEvent + waitFor; two other raw-dispatch tests in that file were checked and are not the same race (one asserts a stable negative on a disabled control, the other needs the raw event's defaultPrevented, which userEvent does not expose), so they were left alone.
…dEntityPicker/FlockPicker (#918) Assessment found no reason FlockPickerDialog needed to exist: the mockup's own fidelity requirement — "All flocks" pinned above the results — is already solvable through the picker's existing slots.paper mechanism (the same one Load More renders through), and the original brief said to reuse the picker in the first place. Dashboard now opens the shared Dialog with FlockPicker inside it (the same nesting ExpensesPage already uses), and a new optional `pinnedChoice` prop threads fixed content above the listbox through NamedEntityPicker's engine — omitted by every other caller, so Daily entry/Feed/Water/Expenses/History are unaffected. The engine's search field also gets the 44px minHeight fix proven on the bespoke dialog, this time applied once for all five callers.
The shared Dialog unmounts its children on close, so every reopen of the Lay-rate flock picker was a fresh engine mount, and Dashboard never told it what scope was already committed — every reopen showed aria-selected="false" on the previously picked flock and aria-pressed="false" on "All flocks", with no indicator of the active scope at all. Fixed by threading `scope` through FlockPicker's existing `controlledCommitted`/`controlledGeneration` props, plus `onClear` (wired to the same reset "All flocks" performs, since seeding a committed entity also surfaces the engine's own footer Clear link, previously dead code with nothing scoped). Checked empirically rather than assumed: this does pre-fill the reopened search field with the committed name, matching every other FlockPicker/CustomerPicker caller's own reopen behaviour, and does not narrow the results, since the seed only touches display text, never the discovery filter. Also corrects the "five callers" count from the prior round's comments to the real total: eleven pre-existing render sites (nine FlockPicker, two CustomerPicker) across seven screens.
1be88e0 to
f1837a3
Compare
|
Rebased onto #927 (Sales order desk). Confirmed before rebasing: it does not touch #929 (Node/npm pin). Reinstalled under the pinned toolchain ( Full suite on the rebased branch (measured now, not carried over from before the rebase): CI is running fresh on |
1 participant
Amendment (2026-09-21) — bespoke picker replaced
The Lay-rate flock-scope picker described below as
FlockPickerDialog(a bespoke, hand-rolled dialog) has been replaced by the sharedNamedEntityPicker/FlockPickerinside the sharedDialog— the same nestingExpensesPage.tsxalready uses.docs/designs/822-mui-revamp.md'sNamedEntityPicker↔Autocompleterow (pair 1, #826/#898) covers this screen: itsslots.papermechanism, already used for the Load-more footer, now also pins "All flocks" above the results via a new optionalpinnedChoiceprop, omitted by every other caller.This is the record of a process failure as much as a feature: three defects fixed in review rounds below — the query-generation race on load-more, the missing initial focus on the search field, and the 37px tap target — all landed in mechanics
NamedEntityPickeralready had, reviewed and hardened under #898 and #512. That is the concrete cost "reused, never rebuilt" is pricing. The 44px fix now lives in the shared engine, benefiting eleven pre-existing production render sites across seven screens — nineFlockPicker(Daily entry, History, Feed ×2, Water ×2, Users, Expenses ×2) and twoCustomerPicker(Sales ×2) — instead of one. (Correction: an earlier version of this note said "all five callers"; the real count is eleven, and this PR's own Dashboard usage makes it twelve.)One divergence from the mockup is left as an open owner decision, not resolved in this PR: the mockup's own hand-rolled picker moves Home/End as list navigation;
NamedEntityPickerdeliberately keeps Home/End as native text-cursor movement (FR-031, serving every other render site above). This PR does not touch that contract.Further amendment (2026-09-21) — reopen-after-select regression, fixed
An independent review found a P2: reopening the picker after picking a flock (or after picking "All flocks") marked nothing as the active scope — the shared
Dialogunmounts its children on close (MUI's defaultkeepMounted={false}), so every reopen is a fresh engine mount, and Dashboard never told it what was already committed. Fixed by threadingscopethroughFlockPicker's existingcontrolledCommitted/controlledGenerationprops, plusonClear(wired to the same reset "All flocks" performs, since seeding a committed entity also surfaces the engine's own footer Clear link, previously dead code). Checked empirically rather than assumed: this does pre-fill the reopened search field with the committed flock's name, matching how every otherFlockPicker/CustomerPickercaller already behaves on reopen — not a regression, since the bespoke dialog this replaced never had a committed-flock concept to seed from — and does not narrow the results, since the seed touches only the display text, never the discovery filter. Four new regression tests inDashboard.test.tsx, each mutation-verified.Full detail and verification: see the PR comments at the 6c561bd and 1be88e0 rounds.
Closes #916
What changed
The bug (step 1, TDD).
dayStrip()inweb/src/lib/dashboard.tsscaled the 14-day strip's bars against the peak of complete days only. When no day in the window was complete — a farm where most houses never file —maxfell straight tonullandheight()floored every bar at 2%, even though the tooltip showed a real figure. Fixed with a fallback: when no complete day exists but at least one day has a recorded figure, scale against the largest partial total instead, and label the caption "partial days only". The complete-day-only average is unchanged. A newscalefield ("complete" | "partial" | "none") onDayStripDatadrives both the caption and (after a runtime-found defect, below) the accessible name.Failing-test evidence before the fix (
web/src/lib/dashboard.test.ts):The feature. The Dashboard's Lay rate card gains a flock scope, per the approved mockup in
docs/designs/916-production-scale/SELECTION.md:NamedEntityPicker/FlockPicker(MUI Autocomplete, feat(web): replace NamedEntityPicker's combobox with MUI Autocomplete #898) — All flocks stays reachable without opening the search at all, which is how this satisfies "All flocks stays available above the scrolling results" without adding a synthetic option inside the shared picker's own result list.getProductionReport(from, to, flockId)call a manual pick of that flock would produce — structural parity between "only one" and "selected single", confirmed both in a unit test and live (the restricted-Worker card and the Owner's card scoped to the same flock render identical figures: 98.8% / Avg 331.6 / Peak 342).Server side.
GET /api/v1/reports/productiontakes an optionalflockId, scoped throughReportQueries.GetProductionAsync(eggs, grade totals, and the hen-day exposure walk narrow together — narrowing eggs without narrowing exposure would rate a healthy flock at a fraction of its real lay, the #780 defect in miniature). The endpoint checksIFlockRepository.GetByIdAsyncfirst and 404s a flock the caller cannot see, reusingFlockEndpoints' own existence-check pattern.Three defects found by end-to-end browser evidence, not by the unit suite, fixed in the same PR:
phone.spec.ts's standing guard went red against it. Bumped to 44px.onCommitnever closed the search, unlike every otherFlockPickercaller in the app (Daily entry, Feed, Water, Expenses, History). Fixed to close on commit.trendLabel's accessible sentence branched onmax === null, which the scale fix already made true for both "no data" and "partial fallback" — so a screen reader was told "no peak or average" over a window whose visible caption and Peak figure showed a real number. Rewritten to branch onscaledirectly; the now-unreachabletrendStripLabelNoCompletekey is removed.Docs. A GLOSSARY.md entry for the strip's scale fallback and for the flock scope, an in-app glossary entry in en/es/tl, and an addition to the existing Dashboard help text.
Suite totals
npm run typecheck && npm test): typecheck clean, 3141/3141 passing (132 files), including the style guards (styles.bare-elements,styles.declared-tokens,styles.elevation,styles.harness-selectors,farmTheme.policy).dotnet build Cluckwork.sln— 0 warnings, 0 errors.Cluckwork.Domain.Tests491/491.Cluckwork.Application.Tests532/532 (includes the module-ledger, adapter-reach, seam-surface and coupling-matrix guards — the newFlockManagementreach onReportEndpoints.Productionis declared there).Cluckwork.Api.IntegrationTests(Docker) 1854/1854, including three new tests: scoped-vs-farm-wide partition, a flock outside the caller's own flock scope (404), and a flock from another tenant or naming nothing at all (404).cluckwork-sim): new spectools/simulation/ui/specs/dashboard-flock-scope.spec.ts— 4 desktop + 1@phone, 5/5 passing. Full quick smoke suite on that same stack: 62 passed, 1 pre-existing opt-in skip, 0 failures (the 44px guard failure above was found, fixed, and reverified green in this same pass — not left for a re-review round).What's deliberately left to #914/#915
Per SELECTION.md's own "still outstanding" note: the 14-day window itself stays fixed (#914's range switcher integrates later — the flock-scope state is plain page state, not derived from anything that resets, so a range control can sit beside it without clearing the choice). Historical flock eligibility policy and any saved-preference persistence for the scope were flagged there as needing implementation review and are not addressed here.
Evidence
Before/after screenshots and the full capture set are attached in a follow-up comment, naming the head SHA.