trunk-merge/pr-89893/7b6a524f-730c-4910-80ac-7351c56fb637 - #90051
Closed
trunk-io[bot] wants to merge 69 commits into
Closed
trunk-merge/pr-89893/7b6a524f-730c-4910-80ac-7351c56fb637#90051trunk-io[bot] wants to merge 69 commits into
trunk-io[bot] wants to merge 69 commits into
Conversation
Convert five dispatch ladders (isinstance chains, len()+index probing of field chains, nested dict.get() probing) to match statements using class patterns with field capture, or-patterns, guards, sequence patterns, and mapping patterns. Behavior is unchanged: every branch of each old ladder maps to exactly one case, in the original order or one proven equivalent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 3cb06508-6ac6-4768-a330-4495ac21001e
The Databricks events export ships a reduced column set that left out created_at, so users could see when an event happened but not when we created the event row, and had no way to measure how late events arrived. Two separate lists excluded it: the fields we query out of ClickHouse, and the hardcoded Databricks table columns that COPY INTO selects. Both now carry created_at, and a new test asserts they stay in sync. Also skip any table column the staged files do not carry. A run that staged its data before a field was added would otherwise build a COPY INTO referencing a column that is not in the Parquet, and retry on it forever. Generated-By: PostHog Desktop Task-Id: bed039fb-5690-475a-b27d-1b0a6db2b01d
Review found the guard could empty the column list for an events export with a custom HogQL schema, because that export takes the hardcoded events branch even though its columns never matched it. An empty list builds a CREATE TABLE with no columns, inside an activity that retries forever. A partial overlap was worse: the run succeeded and dropped the columns the user asked for. The guard now runs only for the default field set, where the hardcoded list and the staged data are meant to match. Custom schemas keep their current behavior. Move the events columns into `_events_table_fields` so a test can compare them against the queried fields without the guard filtering the result first, and add `created_at` to the pre-created table in the schema-evolution test, which otherwise loses two columns while asserting on one. Generated-By: PostHog Desktop Task-Id: bed039fb-5690-475a-b27d-1b0a6db2b01d
Add a Box plot option to SQL insights, gated behind the sql-box-plot-insight feature flag. Maps one row per X-axis and series pair into grouped boxes, auto-maps conventional names for minimum, percentiles, median, mean, and maximum, and keeps percentile calculation in SQL instead of using limited browser results. Adopted from PR #84558. Rebased onto master to resolve merge conflicts and squashed into a single commit. Generated-By: PostHog Desktop Task-Id: f1e69bea-f4e6-44cf-bd12-dc81f8a435c3
The shared constant wrapped a single field for two call sites, one of which was the pre-existing S3 entry. That did not clear the bar for extraction, and it put churn in a file three destinations read. Databricks now declares the field the way it declares its other columns, and common.tsx goes back to untouched. Generated-By: PostHog Desktop Task-Id: bed039fb-5690-475a-b27d-1b0a6db2b01d
Extract the repeated single-Y-axis boolean into a named isSingleAxisChart flag in the display tab, and reuse one height-class expression for the box plot chart's empty, error, and success states. No behavior change. Generated-By: PostHog Desktop Task-Id: f1e69bea-f4e6-44cf-bd12-dc81f8a435c3
Leave Enter to the hosted survey textarea and rely on the survey SDK Cmd/Ctrl+Enter shortcut for submission. Keep the editor preview hints aligned with respondent behavior. Verified with the external survey backend suite, frontend typecheck, CI preflight, and focused Oxlint and Oxfmt checks.
6 updated Run: 15e93842-9e54-48e9-b856-6eced8bb9c0e Co-authored-by: pauldambra <984817+pauldambra@users.noreply.github.com>
…guard exists The comment on created_at gave the wrong reason. The column matters because `timestamp` is client-supplied and a wrong device clock can put it years off, so users need a server-side ingestion time they can rely on. The guard's comment did not state the mechanism that makes it necessary: staging and copying are separate activities, so a deploy between them leaves the staged files a version behind the column list. Generated-By: PostHog Desktop Task-Id: bed039fb-5690-475a-b27d-1b0a6db2b01d
The replay-vision-alert-state-direct-mutation rule includes every .py file under products/replay_vision/backend, which sweeps in the migrations directory. Migration 0057 disables ReplayScanner rows in bulk, so the rule flags it even though a scanner is not an alert. Migrations act on historical models from apps.get_model and cannot call the state machine, so exclude the directory. Generated-By: PostHog Desktop Task-Id: 148d4a90-f54a-41a5-95d6-f702dd5feac8
Return the raw series label when a trends result does not include action metadata. Generated-By: PostHog Desktop Task-Id: ca36deeb-2df0-4f9b-9d48-ed2f3e987a9a
2 updated Run: 6bf474dd-8b3a-4964-97a9-bafbe2282a55 Co-authored-by: sampennington <56024559+sampennington@users.noreply.github.com>
CI ideas get re-proposed because the evidence against them is scattered across closed PRs and revert commits nobody greps. The same handful keeps coming back: xdist in the shards, BuildKit cache mounts, sharding the E2E suite, coverage-based test selection. Adds docs/internal/ci-things-already-tried.md, keyed by the proposal rather than the symptom, because the lookup happens when someone has an idea, not when they are debugging. Each entry carries the verdict, the date, the measurement, and a line of alternate phrasings so a differently-worded grep still lands. Verdicts are not bans. Every entry states the specific blocker so a reader can check whether it still holds; runner sizes and tooling move. Seeded from Julian's merged, reverted, and abandoned PRs since Oct 2025. Narrowed with local git: revert titles, plus lines added in one PR and removed in a later one across the CI hot files. That cut 847 PRs to 333 candidates, then to the entries here. The five CI skills that would otherwise carry duplicate copies get a one-line pointer instead.
…glish The first draft read like PR prose: idioms, passive constructions, and long sentences that carry two ideas. Agents and non-native speakers both parse that worse, and this file exists to be scanned fast during a lookup. Rewrites to ASD-STE100 writing rules while keeping full vocabulary: active voice, simple tenses, one idea per sentence, consistent terms for the same thing. Now averages under 10 words per sentence with none over 25. No entries, verdicts, or PR links changed.
… doc Paul's original ask in #team-devex named three areas: CI, django perf, and tests. The first pass covered CI and tests only. Adds a Django performance section that points at django-startup-time.md as the deep source, plus the three proposals that keep coming back: squashing the migration history (#48267, effect on timing too small for the blocker work), pydantic defer_build (reverted, relocated cost onto the first query after a deploy), and dropping pydantic from the generated schema (250+ model_validate callers make it non-local; the import cost was solved by evicting the module from django.setup instead). Also adds the Blacksmith runner trial (#54559 to #57991). Nothing in the repo recorded why the shadow existed or why it went away, and compare-ci-runners.py is still sitting there marked legacy. Vendor benchmark numbers stay out. They are unpublished internal measurements of a named third party, and the trial's own conclusion was that cache-state asymmetry, not runner speed, drove the large gaps.
An agent recently read CI, found no pytest-snob in pyproject.toml, and concluded the selector was dead. It isn't: pytest-snob is an inline PEP 723 dependency of tools/snob_backend_test_selection_shadow.py, so it never appears in the dependency file. The narrow scope added to the confusion. Selection ran on drafts only for a while, deliberately, until the merge queue was stable. #85530 extended it to ready PRs and #88265 folded Django and product selection into one job. This is the exact question the doc exists to answer, so it gets an entry rather than a correction in a thread.
PRODUCT_SAFETY_FACTOR was a single 1.3 covering two unrelated risks. On a packed bucket the products run sequentially, so the job's wall is the sum of its parts and there is no mean-versus-max gap to cover. The 30% markup there only bought extra buckets, each paying the base overhead again. On a split product the factor is load-bearing: sizing solves for the mean shard while the run's wall is the max, and nothing else bridges that. Split it in two. Buckets get 1.1, covering durations-map error alone. Splits get a margin interpolated from the measured max/mean at p90, which rises with the shard count because the max is an order statistic over more shards. Resolving that needs a fixed point, seeded at no margin so a product that fits one shard is not split in two by its own headroom. Product test jobs now carry legs, one turbo invocation each with its own pytest-split flags, so a split product's last shard can share a job with whole products without --splits/--group leaking onto them. The packer can seed from those remainders. It does not fire yet: with the margin reserved on every shard there is almost no modeled spare, and the real spare, a trailing-shard taper, is not something the sizer can see today. Landing the mechanism now keeps it inert until the margin drops. DEDICATED_BUCKET_PRODUCTS drops batch-exports. It was listed for an async-fixture teardown hang; it now runs at the median product failure rate with no job near the timeout, and the branch was unreachable regardless since the split path is checked first. The set stays as the extension point, with the criterion for re-adding written down. Also corrects the end-to-end claim in the sizing comment. The pre-shard preamble measures about 5.5 minutes, so a 12-minute shard target puts a full PR run near 19 minutes, not 15. Claude-Session: https://claude.ai/code/session_01EJ1y3UzXR1BKtukE886ks9
The first pass filtered closed PRs to those with a closing comment over 60 characters, which covered 34 of 125. That threw away the stale-bot closures, where the body still records exactly what was tried and why it stopped. Adds the ideas that get re-proposed most: Tilt over mprocs, sharing the dev environment across worktrees (three separate attempts), sparse-checkout on the big CI workflows, moving ee/ into products/, session-scoped django_db_setup, and six unmerged attempts at validating responses against the OpenAPI schema. Two existing entries were wrong by omission and are corrected. BuildKit cache mounts were rejected in bulk, then adopted narrowly for uv, which then cached wheels built against a different libxmlsec1 and broke the image; the fix that stuck puts the library version in the cache ID. Migration squashing had a second attempt on a different design, and that one also stalled. Both corrections came from checking present-day state rather than trusting the verdict in the PR, which is the habit the doc asks readers to have.
…e saga Ran a per-entry check of all 32 entries against PR metadata, PR bodies and comments, and present-day repo state. 29 held up. Three did not, and all three failed the same way: the PR was trusted over the repo. - The Tilt entry claimed bin/mprocs.yaml is still the process list. It is not. bin/start defaults to phrocs, which landed in #50341 back in March and reads a config generated per run by hogli dev:generate. - The SeaweedFS entry dated the first attempt to Mar 2026. The real first attempts were Sep and Oct 2025, the replay store migration merged in Nov 2025, and the PR that actually shipped the objectstorage swap (#60432) was never cited. The one PR it did cite never merged. - The xdist entry quoted 42.8%. No source says that. The closing comment gives rounded wall times, so the entry now gives those. Two claims were mine rather than the record's, and are marked or removed. The Blacksmith trial's duration and its cache-asymmetry conclusion are not in either PR, so the entry now says the detail lives in a team-channel report it cannot show. The sparse-checkout entry stated a rejection reason that nobody ever wrote: no human reviewed that PR, the stale bot closed it. It also described a diff that touched three workflows when it touched two, and missed that ci-rust.yml already used sparse-checkout beforehand. Removes Tilt, SeaweedFS, and Granian. None for being old. Tilt's subsystem was rebuilt around it, and the other two shipped, so neither failure is a live trap. Adds a "Remove an entry" section so the next removal has a rule to cite. The oldest entry in the file, xdist from October, stays: it is still the most re-proposed idea here. Adds the person-table cutover, eight unmerged PRs from November trying five mechanisms to repoint the Person model at a partitioned table. Person access now goes through the personhog client, so a Django-level cutover is no longer where that problem lives.
The timing workflow has failed on every run since .test_durations left the
repo. That PR stripped the commit machinery from this workflow but left a
`git checkout -- .test_durations` behind, and the file is now untracked and
gitignored, so the step errors:
error: pathspec '.test_durations' did not match any file(s) known to git
Consequence: no map has been published since. Every consumer has been
planning against a cache entry from before the duration floor was removed,
so that fix has never actually reached CI. Drop the checkout, and guard the
copy — the cache restore is best-effort and there is no tracked file behind
it any more.
With an unfloored map in hand the fitted split margin is wrong. It was
calibrated against shards planned on a floored map, where tens of thousands
of tiny parametrized tests carried ten times their real weight and the
partition only looked balanced. Replaying the real map through the splitter
puts max/mean at 1.02, not the 1.12-1.32 the table assumes, so the table
would now buy shards nothing needs.
Derive it instead. pytest-split cuts between tests and never inside one, so
a contiguous split's worst chunk runs at most one whole test past the mean.
Sizing the worst chunk rather than the mean gives
work / n + maxTest <= budget -> n = ceil(work / (budget - maxTest))
which needs no fitted constant, no fixed point, and tracks the map rather
than a snapshot of it: a product carrying one heavy test gets the shards
that test forces, an evenly grained one gets none it does not need. A test
at or above the budget is called out on its own — no split can hold the
target then, so size by work alone instead of buying shards that cannot
help.
Against the current map this sizes the split products at 22 shards where
they run 27 today, and stops splitting signals, whose whole suite fits one
shard.
Claude-Session: https://claude.ai/code/session_01EJ1y3UzXR1BKtukE886ks9
…hard-sizing-factors
The drift check compares the depot shadows against canonical, so the legs loop and the untracked-map guard have to land in both. Claude-Session: https://claude.ai/code/session_01EJ1y3UzXR1BKtukE886ks9
…hard-sizing-factors
Review of the previous commit found the drift fallback still assumed the previous map is always on disk. Tolerating a cache miss in "Set the previous map aside" opened a path where /tmp/previous_durations does not exist, and the fallback's jq then aborts the whole processing step under bash -e, so Core, Temporal and Dagster never publish either. Losing the product entries for one refresh is the smaller failure, so fall back to an empty slice and keep the rest. A product test job now refuses to pass when its matrix entry carries no legs. An unset or malformed entry left the loop unentered and the job green having run no tests, which the single-command form could not do, and the rollup gate only reads this step. Three comments were left describing behaviour the code no longer has: the split path claiming a safety factor it no longer applies, a restore step claiming a cache miss leaves a checked-out file, and a packing note claiming optimal_chunks leaves the last shard lightest. The algorithm splits its heaviest group to reach the requested count, so that last claim was never true; the packing is sound because work/shards + maxTest bounds every shard, which the comment now says instead. Claude-Session: https://claude.ai/code/session_01EJ1y3UzXR1BKtukE886ks9
The mean-plus-one-test bound stops being informative once a single test eats most of a shard's budget: the denominator collapses and the count runs to the cap. A product with 321 seconds of work and one 319-second test asked for 50 shards where 3 hold the target. Below a quarter of the budget, size by work and add the one shard that carries the heavy test. The bucket factor's comment described the shared factor the sizing used before, which is history rather than a reason and goes stale as the model moves. It now states the invariant alone: the bucket factor covers error in the recorded durations, and the split path derives its own headroom. Claude-Session: https://claude.ai/code/session_01EJ1y3UzXR1BKtukE886ks9
The Person-table entry said `PERSON_TABLE_NAME` is not in the code. It is: #41620 merged from the same window and carried the setting to master, where `Person.Meta.db_table` reads it. The entry steered readers away from a switch that already exists. What did not land is the test setup and dual reads around it, so the entry stays and now says that. The xdist entry drew a 3-minute conclusion from a 6-minute measurement. The closing comment on #38927 reports ~15 min to ~9 min. The Blacksmith entry paraphrased a conclusion that lives only in an internal report. This repo is public, so the entry now carries the outcome the PRs show and stops there. Sparse checkout was labeled `rejected`, which this file defines as built and measured. Nobody reviewed that PR and the stale bot closed it, so `abandoned` is the verdict the taxonomy already has for it. Pointers went only to five CI skills, so a migration, endpoint, isolation, or stacking proposal never saw the sections written for it. Four more skills get the same one-line pointer. Claude-Session: https://claude.ai/code/session_011D2GDLMQGPxQLEkHxevuRs
Make severity controls, filters, and settings available without the rollout flag. Remove the retired flag constant and update affected tests and stories. Generated-By: PostHog Desktop Task-Id: e5e0d821-a9c7-4b96-9ade-55c378a59fbc
8 updated Run: be75d635-fd5e-43e9-9022-76cd056ea9f0 Co-authored-by: hpouillot <3455883+hpouillot@users.noreply.github.com>
4 updated Run: 40e5c001-3352-49ba-9030-f30f9f5ebde8 Co-authored-by: hpouillot <3455883+hpouillot@users.noreply.github.com>
trunk-io
Bot
deleted the
trunk-merge/pr-89893/7b6a524f-730c-4910-80ac-7351c56fb637
branch
August 27, 2026 12:12
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request was created and is being managed by Trunk Merge.
This pull request is based on the master branch at SHA 7eced17f1684d6461a693c4621acaa75336840e9.
See more details about each PR in the batch here:
When CI completes, this pull request will be closed automatically.
Pull Requests Being Tested
This pull request is testing a batch with the changes from pull requests 89893 and 89666 - batching documentation.
Dependencies
This pull request depends on the changes from pull requests 89894, 89886, 89868, 89869, 89755, 89945, 89958, 90004, 84558, 90010, 83663, 89870, and 89835.