fix(stand): date-independent journeys + seed; e2e-stand runs on PRs from the checkout - #2453
Conversation
…ndent The two evidence drill-down journeys clicked the FIRST body row of a timeseries table. The first row of a trailing-month window is the leading partial week bucket, whose overlap with the window can be a single day — and a day with no activity renders "—" with no button (the frontend only wraps a valued cell in one). Whether that click ever lands therefore depended on which calendar day the run started, which is why the stand lane went red on some dates and not others. Two sides of the fix: - Page objects target the first DRILLABLE cell: open_bucket_evidence and open_total_row_evidence click the row group's first button, and open_first_commit_bucket filters to the first row whose Commits cell carries one, staying pinned to the Commits column so the dialog opened is the one named. - The git seed floors commits at one per person-day (weekday_multiplier still shapes volume), so every bucket of the git surface is drillable whatever day the window starts on. PROFILE.md regenerated for the new seed_revision. Verified against a freshly re-seeded compose stand: the two previously failing journeys pass and the full suite is green (296 passed, 3 xfailed, 2 xpassed — the xpasses are the known #2361 flake entries). Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
📝 WalkthroughWalkthroughThe seed generator now creates at least one commit per eligible person per day. UI page objects now locate drillable evidence controls across partial or empty leading buckets and footer rows. ChangesSeed generation and evidence navigation
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ingestion/tools/seed/insight_seed/generators/git.py`:
- Around line 91-97: Apply the same minimum-one commit-count rule used by
seed_class_git_commits to seed_class_git_pull_requests_commits, preferably by
reusing a shared helper, so zero Poisson draws still generate the commit index 0
and its PR link. Add a regression test covering a zero Poisson draw and
confirming the commit and link are created.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc461118-8d01-4b5c-be9e-ab18b9ca8595
📒 Files selected for processing (4)
src/ingestion/tools/seed/PROFILE.mdsrc/ingestion/tools/seed/insight_seed/generators/git.pytests/stand/ui/pages/group_dialog.pytests/stand/ui/pages/person_view.py
|
Dispatch run on this branch: 31567069878 — api-smoke ✅, ui-journeys 16/17 with The ui-journeys lane runs the suite baked into the published
The page-object fix is proven on a live stand by a host-side run of this branch's code: both previously failing journeys pass and the full |
…nd and tests Mirror the changes job's relevance regex as an explicit paths: list on the pull_request trigger, and broaden both in step: any change under tests/, src/backend/ or src/frontend/ (not just tests/stand, tests/lib and the frontend helm chart) now counts as stand-relevant. On pull_request every job still skips, so the filter only prunes no-op runs from the Actions tab; the changes job keeps doing the same filtering for merge_group, which supports no paths: key. While the filter exists, Stand E2E can only be required on merge_group, never at PR level - a required check whose workflow never triggers reports as pending forever. The comments record that constraint. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The commit floor broke the RNG re-derivation contract: seed_class_git_pull_requests_commits still computed the day's commit count with the raw Poisson draw, so on a zero-draw day class_git_commits carried a floored commit while the links table skipped the day entirely - and any PR created that day lost its author attribution in gold's pr_commit_emails join. Both generators now call one _daily_commit_count helper. The regression test also surfaced the adjacent shortfall: a day with fewer commits than PRs dealt empty slices to the tail PRs, with the same linkless consequence. The round-robin now wraps on short days - a link per PR outranks never double-linking a commit. Unit tests lock the parity at the emitted-row level and pin the anchor so the fixture window provably contains a zero-draw PR day. PROFILE.md regenerated (seed_revision tracks the generator source). Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The lanes previously skipped on every pull_request by design - the stand gated only the merge queue, nightly and dispatch, and a PR run existed solely so the check could report a skip-as-pass. The stand must go with any change it covers, so the event guard is gone: a PR whose diff matches the paths filter now runs api-smoke and ui-journeys for real, with superseded runs cancelled by the concurrency group. test-stand up refuses a tree that changes src/backend/ rather than testing it against main's pulled images, so the changes job now also reports whether the diff touches the backend; the lanes turn that into up --build-backend and a wider timeout. Also: the seed package's new link-parity tests are reworked to pytest idiom. pytest stays out of the published seed image - build-images.yml installs it transiently into the throwaway validation container, which still runs the tests against exactly the published package. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Pull request was converted to draft
The ui-tests image bakes the suite in and :latest only moves on a main push, so image mode on a PR ran main's old test code against the PR's stand - a lane that cannot go green on the very PR that fixes it, and a red that stops meaning anything. PR, merge-queue and dispatch builds now run the journeys host-side from the checkout (the stand publishes the gateway on the runner's localhost, which satisfies the __Host- cookie origin constraint the netns join existed for). The nightly keeps image mode: it runs on main, where the baked suite and the checkout agree, so the published runner itself stays proven daily. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The fallback existed for the bootstrap - the first run before a main push had ever tagged :latest. The image has long been published, the lane is nightly-only now, and its whole point is to prove the runner that was actually published; building a local substitute on a failed pull would test something no developer pulls. A failed pull now fails the lane loudly, whatever the cause. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
e2e-stand.yml runs the journeys from the checkout on PR builds and consumes the ui-tests image solely via the :latest a main push publishes, so the PR-side build proved an artefact nothing reads. A broken ui-tests.Dockerfile now surfaces on the main push after merge instead of on the PR - accepted, the image is nightly-parity-only now. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Its last consumer was CI validating the image itself: after the ui-journeys lane moved to running the suite host-side from the checkout, nothing pulled it - and a published artefact whose only reader is the pipeline that publishes it is maintenance without a customer. Removed: the Dockerfile, the build/merge jobs and ui_tests change filter in build-images.yml, the nightly image mode and image pull in e2e-stand.yml (the nightly now runs host-side like every other event), and the version-sync bookkeeping in tests/versions.yaml. Docs and dev-compose help updated; the generic --image mode of test-stand test remains for a locally built runner. insight-ui-tests stays listed in ghcr-cleanup so already-published versions keep getting pruned. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The file was a hand-maintained sync ledger whose job was keeping the pip playwright pin, the Dockerfile base tag and the image's baked browser in agreement - and the ui-tests image removal took the last outside party with it. Nothing reads the file; what remained duplicated tests/pyproject.toml. The one note worth keeping (workflows' 3.12 setup-python vs the suite's 3.13 floor) moved into the pyproject comment beside the floor it annotates. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
What
The two evidence drill-down journeys (
test_git_commit_bucket_opens_and_exports_supporting_data,test_table_total_opens_supporting_data_for_the_whole_period) fail on some calendar dates and pass on others — they are red in the current nightlies and in dispatch run 31560385144.Why
Both page objects click the first body row of a timeseries table. The first row of a trailing-month window is the leading partial week bucket, whose overlap with the window can be a single day. A bucket with no activity renders
—, andMetricTableValueonly wraps a valued cell in a<button>— so on those dates there is nothing to click and the locator times out after 30 s. The seeded activity is weekday-shaped and Poisson-drawn, so whether the leading bucket is drillable depends on which day the window starts (weekend-only overlap being the common empty case).Fix
Page objects target the first drillable cell instead of the first row:
open_bucket_evidence/open_total_row_evidenceclick the row group's first button;open_first_commit_bucketfilters to the first row whose Commits cell carries a button, staying pinned to that column so the dialog opened is the one the test names.Git seed floors commits at one per person-day (
weekday_multiplierstill shapes volume), so every bucket of the git surface is drillable whatever day the stand is seeded.PROFILE.mdregenerated for the newseed_revision.Seed link parity (review follow-up): the commit floor had broken the RNG re-derivation contract —
seed_class_git_pull_requests_commitsstill computed the day's commit count with the raw Poisson draw, so a zero-draw day carried a floored commit with no PR-link rows, and a short day (fewer commits than PRs) dealt empty slices to the tail PRs; either way the PR loses its author attribution in gold'spr_commit_emailsjoin. Both generators now share one_daily_commit_counthelper and the round-robin deal wraps on short days.tests/test_git_links.pylocks the parity at the emitted-row level, with the anchor pinned so the fixture window provably contains a zero-draw PR day.CI workflow (absorbed from #2458, then extended)
e2e-stand.ymlis converted from a merge-queue-only gate into a per-PR one, and the stand's scope is broadened to any change undertests/**,src/backend/**, orsrc/frontend/**:pull_requestis gone: a PR whose diff matches the newpaths:filter (mirroring thechangesjob's regex — the two lists are documented as kept in sync) runsapi-smokeandui-journeys, with superseded runs cancelled by the concurrency group.merge_group(nopaths:support), the nightly, and dispatch keep working via thechangesjob. While the paths filter exists,Stand E2Ecan only be made required onmerge_group, never at PR level — a required check whose workflow never triggers reports as pending forever; the current main ruleset requires neither, so nothing breaks today.test-stand uprefuses a tree that changessrc/backend/rather than testing it against main's pulled images, sochangesreports abackendflag and the lanes translate it intoup --build-backendplus a wider timeout.insight-ui-tests:latestimage, which only moves on a main push — a PR lane therefore ran main's old test code against the PR's stand and could not go green on the very PR fixing it. The host'slocalhost:<port>satisfies the same__Host-cookie origin constraint the image's netns join existed for.tests/versions.yamlsync ledger): after checkout mode, its only consumer was the pipeline validating the image itself.test-stand test --image <ref>remains as a mechanism for a locally built runner;insight-ui-testsstays in ghcr-cleanup so published versions keep getting pruned.Verification
Against a freshly re-seeded compose stand: both previously failing journeys pass, and the full
tests/standsuite is green — 296 passed, 3 xfailed, 2 xpassed (the xpasses are the known #2361 flake entries).This branch's UI-test code was also run host-side against a live stand seeded without the commit floor on a date where CI reproduced the flake — 17/17 journeys pass, proving the locator half absorbs the undrillable leading bucket on its own. And the converted workflow demonstrates itself on this PR: the PR-triggered
e2e-standruns execute both lanes from this branch's checkout, 17/17 ui-journeys,Stand E2Egreen.Summary by CodeRabbit
Bug Fixes
Documentation