Repository navigation
Conversation
bun run db:seed never called faker.seed(), so two runs with the same arguments produced different data and couldn't be diffed for screenshot comparisons. Added a --seed CLI flag (default 42) applied before any faker call, including the module-level USER_POOL/ SESSION_POOL generation. Paths were also picked independently per event, so any funnel or goal built on the seeded data showed near-zero, meaningless conversion. Replaced that with a weighted page-transition table and per-session path tracking, so a stable share of visitors who view one page go on to view a related next page. Verified determinism (two runs, same --seed, byte-identical ClickHouse rows) and funnel realism (pricing->signup conversion 24.9% vs 16.8% baseline, signup->dashboard 21.7% vs 12% baseline) against local Postgres/ClickHouse.
|
@FindMalek is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
@izadoesdev this is ready for review whenever you have a chance — a partial slice of #969 (the two objective seed-script bugs, not the scenario-engine proposal). CI should be green aside from Vercel preview-deploy authorization, which needs a team member's click. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Must run before any faker call below so the whole run is reproducible. | ||
| faker.seed(seedValue); |
There was a problem hiding this comment.
If the same --seed is used at different times, faker.seed() repeats the random choices, but faker.date.recent() still uses the current date as its reference. Session and event timestamps therefore change between runs, and the existing Date.now() value for each event's created_at changes too. As a result, the flag cannot produce reproducible stored rows. Give both clock-dependent fields fixed values for a given seed.
| interface PathTransition { | ||
| path: string; | ||
| weight: number; | ||
| } |
There was a problem hiding this comment.
The new PathTransition interface violates the applicable TypeScript directive, “Don't use user-defined types.” Infer the transition shape from the data instead. This repository requirement needs to be satisfied before merging.
Context Used: Ultracite Rules - AI-Ready Formatter and Linter (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // A page_exit event exits the page currently being viewed rather than | ||
| // jumping to a new one; only screen_view events advance the journey. | ||
| const path = | ||
| !isNewSession && isPageExit && currentPath | ||
| ? currentPath | ||
| : nextPath(isNewSession ? undefined : currentPath); |
There was a problem hiding this comment.
The new flag parsing and session-based path selection have no automated tests. The manual sample runs do not protect against later regressions in flag placement, repeatability, or reusing the viewed path for page_exit. Focused tests would make those new behaviors safer to maintain.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
bun run db:seed never called faker.seed(), so two runs with the same arguments produced different data and couldn't be diffed for screenshot comparisons. Added a --seed CLI flag (default 42) applied before any faker call, including the module-level USER_POOL/ SESSION_POOL generation. Paths were also picked independently per event, so any funnel or goal built on the seeded data showed near-zero, meaningless conversion. Replaced that with a weighted page-transition table and per-session path tracking, so a stable share of visitors who view one page go on to view a related next page. Also fixes created_at, which was an unrelated Date.now() call per event rather than tracking when the event conceptually happened; it now equals the event's own time. Extracted the pure flag-parsing and path-selection logic into seed-helpers.ts with unit tests, since seed.ts itself is a side-effecting top-level script with no exports. Verified determinism (two runs, same --seed, byte-identical categorical/behavioral columns across 300/300 rows) and funnel realism (pricing->signup conversion 24.9% vs 16.8% baseline, signup->dashboard 21.7% vs 12% baseline) against local Postgres/ ClickHouse. Absolute timestamps intentionally still track real invocation time (faker.date.recent's implicit refDate), not just the seed, so seeded data stays recent enough to show up in the dashboard's default date range regardless of when the script runs; only the categorical/behavioral fields are seed-reproducible.
|
fixed the interface (moved to inferred types via as const) and added real tests for the flag parsing and path selection, pulled into a separate seed-helpers.ts since seed.ts itself is a side-effecting script with no exports. on the timestamp point: fixed created_at, it was an unrelated Date.now() call per event instead of tracking the event's own time, now it does. absolute timestamps across separate runs still won't match though, and that's on purpose - faker.date.recent() anchors to real invocation time so seeded data stays recent enough to actually show up in the dashboard's default date range no matter when you run the script. pinning that to a fixed clock would trade away the main reason this script is useful for local dev. verified: two runs with the same seed produce 300/300 byte-identical rows on every categorical/behavioral column (path, browser, os, session grouping), just not the wall-clock columns. |
|
@greptile review |
|
closing - #1081 completely replaced the seed script with a proper deterministic workspace generator (bun run workspace), which covers this and a lot more. nice work on that, good to see it landed. |
1 participant
Addresses part of #969
Problem
bun run db:seedhas two real bugs, separate from the larger scenario-engine proposal in #969:faker.seed(), so two runs with the same arguments produce different data — no way to diff output for screenshot comparisons or reproduce a specific dataset.Fix
--seed <n>CLI flag (default42), applied viafaker.seed()before any faker call, including the module-levelUSER_POOL/SESSION_POOLgeneration. Existing positional usage (db:seed <WEBSITE_ID> [EVENT_COUNT]) is untouched — the flag is stripped fromprocess.argvbefore positional parsing so both forms compose.Verification
bun run check-types(36 packages) andbun run lint(ultracite + lint:policies): clean.packages/db's own test suite: 115 pass, 0 fail.--seed, different website ids, then queried ClickHouse for the full event row shape ordered by time — 300/300 rows byte-identical between runs./pricingthen later/signupin the same session: 24.9% (686/2752), vs. 16.8% baseline/signupvisitation across all sessions./signup→/dashboard: 21.7% vs. 12% baseline. Confirms funnels built on these paths now show non-degenerate conversion.Explicitly out of scope
This is two objective bug fixes to the existing script, not the scenario-engine redesign proposed in #969 (SaaS journey model, custom events, revenue, profiles, uptime/alarms, flags, links, or reconciling with the separate e2e ClickHouse generator). That's gated on an open product question in the issue ("Is SaaS the right story, or e-commerce?") only the maintainer can answer.
packages/db/src/seed.tsalso still has no DB-target safety guard of its own (packages/db/src/test-env.ts's loopback guard is scoped to test bootstrap, not this script) — adding one is part of that larger proposal, not one of the two bugs here, so I left it out; verification above was done againstlocalhostonly.Disclosure: implemented with AI assistance (Claude Code); verified and tested by hand — ran the seed script against local Postgres/ClickHouse, confirmed byte-identical output across two runs with the same
--seed, and confirmed pricing→signup and signup→dashboard conversion lift via ClickHouse queries before submitting.Summary by cubic
Makes
bun run db:seeddeterministic and gives seeded visitors realistic page-to-page sequences so funnels and goals built on seeded data show meaningful conversion.--seed <n>CLI flag (default42) applied viafaker.seed()before any faker call, so two runs with the same arguments produce identical data. Existing positional usage is untouched.created_atto use the event's own time instead of a per-eventDate.now()call, and moves flag parsing and path selection intoseed-helpers.tswith unit tests.Written for commit a09587e. Summary will update on new commits.