fix(e2e): repoint the canary at the markup two PRs replaced by mforce · Pull Request #844 · mforce/cluckwork

mforce · 2026-09-14T03:44:10Z

Closes #841

The canary probe asserted against SPA markup two merged PRs replaced. Two of its four screens could not pass, on any branch, and nobody saw it because the canary step is workflow_dispatch-only — the pull_request run of e2e-smoke.yml skips it. Run 34787979138 failed on the same head SHA whose pull_request run was green.

dashboard. ready was .capture-tile and the populated-rows check was ready.locator("tbody tr"). Since #654 a .capture-tile is a <Link> holding three <div>s, and the dashboard carries no table at all — tiles, a trend chart, a stock <ul>, a sales <ul>. The count was 0 forever. ready is now the .capture-grid and the rows are its tiles, which is what a lost /api/v1/flocks empties.

history. selectOptionContaining needs <select><option>; #642 replaced that filter with a searchable FlockPicker. It now commits through commitNamedPicker, the helper specs/manager.spec.ts:129 already uses on this exact control.

Each screen now names its own rows rather than sharing one hardcoded tbody tr, so a screen whose rows are not table rows can say so instead of failing silently at zero.

Verification

Local sim stack rebuilt from this branch's source and reseeded (tools/simulation/reset.sh), then bash tools/simulation/ui/run-canary.sh.

Before, reproducing CI exactly:

✘ dashboard ... (1.1m)   Error: dashboard rendered its table with no rows against a populated fixture
                         Locator: locator('.capture-tile').first().locator('tbody tr')   Received: 0
✘ history ... (1.0m)     Error: no <option> containing "Sim House A" — the list did not load...
                         Locator: getByLabel('Flock').locator('option')...              Received: 0
2 failed, 2 passed (2.2m)

After:

✓ dashboard (1.7s)  ✓ stock (1.6s)  ✓ reports (1.6s)  ✓ history (2.1s)
4 passed (8.4s)

The history measurement is restored, not just its assertion: canary-vitals/history.json records interactionCount: 26 and longestInteractionMs: 40, so the picker's clicks produce the Event Timing entries yieldsEventTiming: true demands. Mutating the dashboard row locator to .capture-tile-MUTANT turns that screen red with Received: 0, so the new assertion is load-bearing rather than green by construction.

Not fixed here

The canary stays dispatch-only, so the next markup change breaks it the same silent way. Gating it on pull_request would cost roughly two minutes on a job that already builds and seeds the stack. That is a CI-cost decision, not part of this fix.

Summary by CodeRabbit

Bug Fixes
- Improved canary load-test validation across dashboard, stock, reports, and history screens.
- Updated history picker interactions for more reliable filtering.
- Corrected populated-content checks to use the appropriate row or tile elements for each screen.

The dashboard screen asserted `ready.locator("tbody tr")` against a `.capture-tile`, which has held three divs and no table since #654, so the count was 0 forever. The history screen drove its flock filter through `selectOptionContaining`, and #642 replaced that `<select>` with a searchable FlockPicker that renders no `<option>` at all. Each screen now names its own rows, because they are not all table rows. History commits the picker through `commitNamedPicker`, the helper the smoke suite already uses on this same control. Nothing caught it because the canary step is workflow_dispatch-only: the pull_request run of e2e-smoke.yml skips it, so both PRs merged green. Closes #841

coderabbitai · 2026-09-14T03:44:18Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 96d19759-9ef2-4a9e-98d6-b89f64089fea

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: f026f14e-dda9-4ce4-83de-0aad0bda628d

📥 Commits

Reviewing files that changed from the base of the PR and between 6231b31 and cf288a8.

📒 Files selected for processing (1)

tools/simulation/ui/specs-canary/canary.spec.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The canary spec now uses screen-specific populated-content locators. Dashboard checks use capture tiles, table screens use table rows, and history uses commitNamedPicker for the flock filter.

Changes

Canary markup alignment

Layer / File(s)	Summary
Screen-specific populated locators `tools/simulation/ui/specs-canary/canary.spec.ts`	The dashboard, stock, reports, and history screens define locators for their populated content. The history filter now uses `commitNamedPicker`.
Populated-content assertion `tools/simulation/ui/specs-canary/canary.spec.ts`	The assertion calls `screen.rows(ready)` and reports the screen that has no rows.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to cf288

The canary selectors and history picker interaction align with the described current markup. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, conventional, and accurately describes updating the E2E canary for replaced markup.
Description check	✅ Passed	The description explains the problem, changes, scope limits, and verification results. It omits the template's Checklist section, but the core required information is complete.
Linked Issues check	✅ Passed	The PR satisfies the coding requirements in issue `#841`. `canary.spec.ts` defines a separate `rows` locator for each screen, uses `.capture-tile` for dashboard population checks, keeps `tbody tr` for t…
Out of Scope Changes check	✅ Passed	The change is limited to the canary spec and directly supports issue `#841`. The PR does not change the dispatch-only workflow configuration, which the issue identifies as outside this fix's scope. No u…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/841-canary-spec-drift

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

mforce · 2026-09-14T03:44:21Z

@coderabbitai review

coderabbitai · 2026-09-14T03:44:27Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…teract Codex review of the first commit found three holes, two of them live. The dashboard's per-panel failure is a plain `<p class="error">`, with no role, so the `getByRole("alert")` check walked past a panel whose fetch had failed. The error check now counts `.error` as well, and aborting `/api/v1/stock` turns the dashboard red where it was green before. Nothing re-checked a screen after its interaction, so a filter that came back empty or failed left the canary green on the pre-interaction state. Both checks now run twice, and filtering history to a flock with no entries fails with "history rendered no rows after the interaction". The dashboard also checks all three of its panels' empty states, not just the flock one: a tile renders for a flock that filed nothing today, so the tiles alone cannot tell a working stock or sales read from a failed one.

…ds landed Codex round two, three findings, all against the previous commit. Expanding a stock grade whose lots come back empty rendered the heading beside `noLotsMessage` and passed, so that screen now checks the lots empty state too. Rewriting the lots response to `[]` turns it red. Reports rendered its production table before the three money reads, so the post-interaction check could finish while one of them was still in flight. The interaction now waits for the last read and asserts the money section is ON the glass, which is positive rather than an absence a 500 can win a race against. A profit read delayed 1.5s and answered 500 turns it red. Report rows also never proved production: ReportQueries emits a row per calendar day whether or not anything was recorded, so zero-filled rows satisfied the count. The period total is asserted non-zero; it reads 20,946 on the seeded fixture.

…he specs' window Codex round three, against my own previous commit: the non-zero report total assumes the fixture's production lands inside the 30-day window the canary widens to, and SimulationDataSeeder persists its ORIGINAL anchor and reuses it on a re-seed. An old database therefore stays old, a healthy backend answers zero for that window, and the canary would have read as a reports regression. The preflight now asks for the same window and fails with the fixture as the named cause, which is the distinction that file exists to make. It prints the figure it found (20,946 eggs) so the canary's own assertion has a number beside it. Querying 400 days back turns it red. The window is taken in FARM time, not UTC: a farm west of UTC rejects a `to` that is still tomorrow there, which is a 400 rather than an empty report.

…ming entry Real clicks are necessary for an entry, not sufficient: vitals.ts observes at durationThreshold 16, so a sub-frame click is never reported. The comment now says the check holds on margin, with the 26 interactions the quiet baseline records, rather than by construction.

mforce · 2026-09-14T04:13:13Z

Four rounds of codex review / codex exec against this branch, each one re-reading the diff after the previous round's fixes. Five findings confirmed and fixed, all verified by mutation against the local sim stack.

Round 1 (codex review --base main) returned no findings. The adversarial rounds that followed did.

Round 2. The dashboard's per-panel failure is a plain <p class="error"> with no role, so getByRole("alert") walked past a panel whose fetch had failed — aborting /api/v1/stock was green before and is red now. Nothing re-checked a screen after its interaction, so a filter that came back empty left the canary green on the pre-interaction state; both checks now run twice and filtering history to a flock with no entries fails with history rendered no rows after the interaction.

Round 3. Expanding a stock grade whose lots return [] showed the heading beside noLotsMessage and passed; rewriting the lots response to empty now turns it red. Reports rendered its production table before the three money reads, so the post-interaction check could finish while one was in flight — the interaction waits for the last read and asserts the money section is on the glass, which is positive rather than an absence a 500 can outrun; a profit read delayed 1.5s and answered 500 turns it red. Report rows also never proved production, since ReportQueries emits a row per calendar day regardless, so the period total is asserted non-zero.

Round 4, against that last fix. The non-zero total assumes the fixture's production lands inside the 30-day window, and SimulationDataSeeder persists its original anchor and reuses it on a re-seed — so an old database stays old, a healthy backend answers zero, and the canary would have read as a reports regression. The preflight now asks for the same window and fails with the fixture named as the cause, which is the distinction that file exists to make. It prints what it found (20,946 eggs) beside the canary's own assertion. Querying 400 days back turns it red.

Round 5 produced no product defect: two repeats of findings declined with reasons above, and one correct note that the picker comment overclaimed — real clicks are necessary for an Event Timing entry, not sufficient, since vitals.ts observes at durationThreshold: 16. The comment now says the check holds on margin, with the 26 interactions the baseline records.

Declined, with reasons. The dashboard can still pass when today's entries are empty: a tile renders for a flock that filed nothing, and requiring today's entries would be wrong because the fixture deliberately seeds up to yesterday. The reports money figures are asserted present but not non-zero, because a non-zero assertion there would couple to order dates the way the eggs one coupled to the anchor — the trap round 4 just closed.

Verification each round: bash tools/simulation/ui/run-canary.sh against a stack rebuilt and reseeded from this branch (4 passed), plus the 41-test smoke suite for the shared preflight change (41 passed, 1 skipped).

## What this is Measurement for #839, plus one small scheduling change. **Read the numbers before the title's verb** — this does not make the suite faster. It stops the suite sometimes being much slower than it needs to be. `docs/plans/839-integration-wall-clock/measurement.md` is the deliverable. The code is two files. ## The measurement Local, Ryzen 5 6600H / 12 logical CPUs / 18.8 GiB, cached images, Docker 29.8.0, .NET SDK 10.0.112. | Run | Config | Wall | Tests | | --- | --- | ---: | --- | | `main` (`18b45dc`) | Debug | 320s | 1,801 passed | | this branch | Debug | 326s | 1,815 passed | | this branch | Release | 341s | 1,815 passed | Full-suite counts differ because the branch is rebased past #820 and #844. **What the ~5.5 minutes is made of** (instrumented run, 331.5s wall): - **Container readiness: 156.6 distinct wall-clock seconds**, overlapping test execution. 119 Postgres + 7 Redis + 1 Ryuk. - **The shared `integration` collection: 1,024 of 1,815 tests**, one fixture, running serially end to end. - **Three one-shot-process classes dominate**: `SeedCommandTests` 131.14s, `ProcessRoleGuardTests` 127.26s, `OneShotVerbMinimalConfigTests` 119.05s. Repeated app process startup, not SQL. ## Two premises in the issue did not survive 1. **Container reuse is already done.** Lever 1 assumed per-class startup was the dominant cost. The shared collection already holds one `ICollectionFixture<CluckworkWebApplicationFactory>` covering 1,024 tests. The remaining 119 Postgres containers are the specialized factories that genuinely need their own database, migration state, or advisory-lock behaviour. 2. **The remaining cost is subprocess, not SQL.** The issue lists container reuse and parallelism. The measured cost is repeated one-shot process startup in three classes, which the issue never names. ## The change `IntegrationCollectionOrderer` puts the shared collection first and delegates every other collection to xUnit's default orderer. Fixture ownership, concurrency limits, serialization, and the race assertions are untouched — `StealLossConnectionReleaseTests` keeps its dedicated factory, one-slot pool, and timing assertions. Why it is **variance, not speedup**: xUnit 2.9.3 documents its default collection order as unstable between runs. When the 95-class serialized collection draws a late slot, its ~300s of serial work becomes a tail nothing else can overlap. Pinning it first removes that schedule. It makes no test cheaper. The first draft of this PR claimed 25.8% (434.51s → 322.46s). That compared against a single slow run. A second run of the **unmodified** scheduler came in at 327.06s, and the paired runs above put both versions inside each other's spread. The correction is in the measurement doc, not quietly dropped. ## Also in here - `tools/test-timing/` — `measure.py` captures wall clock, TRX, and Docker events; `summarize.py` reports per-class cost, container readiness, and fixture phases. Opt-in timing in the base factory behind `CLUCKWORK_TEST_TIMING=1`. - Two guard collisions found while building the tooling, both fixed in the tooling, **neither guard weakened** — worth knowing because they will bite the next person who adds a file under `tools/`: - the `#508` tracked-file image-pin guard read a Python dict key as a live image reference (fixed by naming the variable `reference`); - the same guard's bare-`postgres:` literal detector fired on a timing phase *label* (fixed by naming the phase `container`). ## Verification - Solution build: 0 warnings, 0 errors. - Integration suite on this branch: **1,815/1,815 passed** in Debug and Release. - Domain 491/491, Application 290/290. - `tools/test-timing/measure.py` driven end to end on the branch as it now stands: exit 0, 1,815 passed, summary regenerated. - CI is **not** measured. Local results do not establish a CI improvement or an optimal worker count on hosted runners. ## Not done No test deletion, no SQLite, no higher parallelism, no product change. Remaining levers, in the order the data now suggests: the three subprocess classes; splitting the shared collection after auditing its shared-state assumptions; CI-side measurement. Closes #839

🤖 I have created a release *beep* *boop* --- ## [0.1.2](v0.1.1...v0.1.2) (2026-09-16) ### Features * **data:** standardize business record chronology ([#820](#820)) ([6231b31](6231b31)) * **infra:** optional leader-lease endpoint for pooled deploys ([#869](#869)) ([e9bc6a7](e9bc6a7)) * **sim:** seed a second farm for the README dashboard capture ([#867](#867)) ([de407c6](de407c6)) * **web:** adopt MUI, themed from the farm palette tokens ([#674](#674)) ([#860](#860)) ([6c83c5c](6c83c5c)) * **web:** convert Daily entry to MUI, field-first on the phone ([#888](#888)) ([b66f8b8](b66f8b8)) * **web:** convert the Dashboard and app shell to MUI ([#829](#829)) ([#883](#883)) ([2e94277](2e94277)) * **web:** retire the Slack-blue link colour for ink + a rule underline ([#884](#884)) ([c08f9d8](c08f9d8)) * **web:** serve a per-request CSP nonce so Emotion's styles apply under style-src 'self' ([#874](#874)) ([ba4e6f3](ba4e6f3)) * **web:** visual language theme overrides for the MUI revamp ([#864](#864)) ([#882](#882)) ([0bb6b73](0bb6b73)) * **web:** whole-app MUI baseline, theme policy guard and the [#740](#740) phone action rule ([#823](#823)) ([#871](#871)) ([af565e4](af565e4)) ### Bug fixes * **auth:** fail closed on unresolved flock-scope actors ([#787](#787)) ([#868](#868)) ([16d0350](16d0350)) * **auth:** make farm configuration owner-only ([#870](#870)) ([42f9036](42f9036)) * **e2e:** repoint the canary at the markup two PRs replaced ([#844](#844)) ([18b45dc](18b45dc)) * **i18n:** tl glossary uses the standard passive of ilagay ([#813](#813)) ([20dec10](20dec10)), closes [#738](#738) * **sim:** stop the k6-baseline EXIT trap masking a clean run as failed ([#838](#838)) ([f5ec96f](f5ec96f)) * **web:** declare the rule tokens the Dashboard reads, and guard undeclared custom properties ([#885](#885)) ([5bead1f](5bead1f)) ### Performance * **ci:** start the serialized integration collection first ([#861](#861)) ([1dcc7f6](1dcc7f6)), closes [#839](#839) ### Documentation * **auth:** record the OAuth 2.1 decision for MCP authentication ([#801](#801)) ([0510854](0510854)) * **designs:** MUI revamp design doc, component map, layout system, IA ([#862](#862)) ([da49481](da49481)) * **readme:** recapture the daily entry, reports and sales screenshots ([#865](#865)) ([f18e336](f18e336)) * **specs:** correct the sales_order_items column list in §10.5 ([#812](#812)) ([afe4a02](afe4a02)), closes [#737](#737) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: cluckwork-lockfix[bot] <309265648+cluckwork-lockfix[bot]@users.noreply.github.com>

coderabbitai Bot approved these changes Sep 14, 2026

View reviewed changes

mforce added 4 commits September 14, 2026 03:59

mforce merged commit 18b45dc into main Sep 14, 2026
18 checks passed

mforce deleted the fix/841-canary-spec-drift branch September 14, 2026 04:45

cluckwork-lockfix Bot mentioned this pull request Sep 14, 2026

chore(main): release 0.1.2 #802

Merged

mforce mentioned this pull request Sep 14, 2026

perf(ci): start the serialized integration collection first #861

Merged

Navigation Menu

Repository navigation

fix(e2e): repoint the canary at the markup two PRs replaced - #844

fix(e2e): repoint the canary at the markup two PRs replaced#844
mforce merged 5 commits into
mainfrom
fix/841-canary-spec-drift

mforce commented Sep 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

mforce commented Sep 14, 2026

Uh oh!

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading

Uh oh!

mforce commented Sep 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mforce commented Sep 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Verification

Not fixed here

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

mforce commented Sep 14, 2026

Uh oh!

coderabbitai Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mforce commented Sep 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mforce commented Sep 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading