Repository navigation
Add pnpm seed:perf to build a large realistic perf-test database - #1300
Conversation
The command seeds a dev data dir with ~1,200 threads and ~400k events that match production event payload shapes. A unit test parses every seeded event with the domain stored-event parser so seeded timelines cannot 500. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🚨 SLOP COP 🚨 · I am the Slop Cop. I am now reviewing this pull request for security, code quality, performance, duplication, architecture, and end-to-end behavior. I will post one final review after the parallel checks finish. |
|
|
||
| const dataDir = resolveTargetDataDir(args); | ||
| const prodDataDir = resolveProdDataDir({ homeDir: homedir() }); | ||
| if (resolve(dataDir) === resolve(prodDataDir)) { |
There was a problem hiding this comment.
🚨 slopcop/review — High: canonicalize the target before the production safety check
resolve() compares path text. It does not compare file identities. I confirmed that ~/.BB and ~/.bb share one inode on this host. The case alias passes this check. A symlink alias also passes it. With --reset, either alias deletes the production database and auth.json.
Use realpathSync.native() for the existing data directory. Also reject a database symlink that targets the production database. Add tests for case and symlink aliases.
| emit({ | ||
| type: "turn/input/accepted", | ||
| turnId, | ||
| data: { providerThreadId, clientRequestId: `creq_${rng.idSuffix()}` }, |
There was a problem hiding this comment.
🚨 slopcop/review — High: reuse the request ID for the accepted input
This event creates a new clientRequestId. It does not use the related client/turn/requested.requestId. I checked one fixture with seven requests and seven accepted inputs. None of the IDs matched.
The timeline uses this ID to connect a user message to its turn. The fixture therefore measures pending, thread-level messages instead of accepted turn messages. Create one request ID for each turn and use it in both events. Add a contract test for this relation.
| return weights.map((weight) => | ||
| Math.max( | ||
| 20, | ||
| Math.min(9_000, Math.round((weight / weightSum) * eventCount)), |
There was a problem hiding this comment.
🚨 slopcop/review — Medium: preserve the requested total after the per-thread cap
The 9,000-row cap removes events without moving the excess to another thread. I ran --projects 1 --threads 8 --events 60000. The command created 39,772 events. This result is 33.7 percent below the requested total.
Remove the cap, or move the excess to threads that remain below the cap. Add a test with a clear tolerance for the final event count.
| db.$client.pragma("wal_checkpoint(TRUNCATE)"); | ||
| // Real directories keep the host daemon's git probes from failing when a | ||
| // seeded thread or environment is opened. | ||
| for (const workspacePath of result.projectWorkspacePaths) { |
There was a problem hiding this comment.
🚨 slopcop/review — Medium: create every ready environment path
This list contains only the root project paths. The fixture also records separate managed-worktree paths as ready environments. The default fixture creates many such environments, but the command creates none of their directories.
A host command for one of these threads fails with path_not_found. This conflicts with the note that agents can run after the seed. Return and create all ready paths, or attach live threads only to the root environments.
| db: DbConnection, | ||
| options: SeedPerfFixtureOptions, | ||
| ): SeedPerfFixtureResult { | ||
| const rng = createRng(options.randomSeed); |
There was a problem hiding this comment.
🚨 slopcop/review — Medium: make the documented append mode repeatable
Each call starts the deterministic generator with the same default seed. A second default run creates the same project IDs. The plain inserts then fail with UNIQUE constraint failed: projects.id.
I reproduced this failure after one successful no-reset run. Add a unique run namespace, or require a new seed and document that rule. Add a second-run test for the chosen behavior.
| WHERE thread_search_segments_fts MATCH 'test'`, | ||
| ) | ||
| .get(); | ||
| expect(ftsRow).toBeDefined(); |
There was a problem hiding this comment.
🚨 slopcop/review — Low: assert that the FTS query found rows
A SQLite COUNT(*) query always returns one result row. Therefore, toBeDefined() also passes when the index count is zero. Assert that indexed is greater than zero, or query for one known fixture marker.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5
This adds a tool that fills a practice BB with many pretend projects and chats. It is like a large toy box for speed tests. Some safety and data links need repairs before people use it.
Review
I found two high-severity defects, three medium-severity defects, and one low-severity test gap.
- High: A path alias can bypass the production-data guard. On this host,
~/.BBand~/.bbshare one inode. The--resetoption can then delete the production database and credentials. - High: Accepted input events use unrelated request IDs. The timeline cannot connect seeded user messages to their turns.
- Medium: The 9,000-event thread cap loses events. A 60,000-event test created only 39,772 rows.
- Medium: Ready managed environments point to directories that the command never creates. Host operations can fail with
path_not_found. - Medium: A second default append run fails because it creates the same deterministic project IDs.
- Low: The FTS test checks only that
COUNT(*)returned a row. It does not check that the count is positive.
I added inline comments with the evidence and a repair for each item.
The default performance path looks reasonable. It created 402,298 events in 15.2 seconds. The database used 496 MiB, and the process used 464 MiB at peak. Inserts use bounded batches and short transactions.
I found no command injection, SQL injection, secret exposure, or duplicate performance seeder. Direct bulk writes fit this tool better than production notifier helpers.
For architecture, reuse HOST_ID_FILE_NAME and HOST_AUTH_FILE_NAME from @bb/host-daemon-contract. A typed request-event helper can also prevent the ID mismatch.
Validation passed for the focused Turbo typecheck and all 86 @bb/scripts tests. All pull-request checks pass. The seeded app loaded in a browser, and I opened a seeded thread timeline.
I posted this review as a comment only. I did not approve the pull request or request changes.
…-bb#1300) ## What changed - Add `pnpm seed:perf`, a checked-in command that seeds a dev data dir with a large, realistic BB database: 12 projects, ~1,200 threads, and ~400k event rows in about 12 seconds (~500 MB). - Event payloads match production shapes: turns, reasoning deltas, command output, file-change diffs, token usage, rate limits, search segments (FTS via triggers), and prompt history. - A unit test seeds an in-memory database and parses every seeded event with the production `parseStoredThreadEvent` parser, so fixture drift cannot make seeded timelines return 500s. - Document the workflow in `docs/debugging-and-qa.md`. ## Why Mobile slowness reports only reproduce at scale. This makes a big, reusable fixture one command away, with deterministic `--seed` runs and append mode for jumbo threads (e.g. `--projects 1 --threads 8 --events 60000`). ## Notes - Default target is the checkout's dev data dir; the command refuses to touch `~/.bb`. - Boot dev once before seeding so the fixture adopts the real daemon host id; `--reset` also removes `auth.json` so the daemon re-enrolls under the persisted host id. - Used to produce the mobile perf findings that will land as issues from this investigation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What changed
pnpm seed:perf, a checked-in command that seeds a dev data dir with a large, realistic BB database: 12 projects, ~1,200 threads, and ~400k event rows in about 12 seconds (~500 MB).parseStoredThreadEventparser, so fixture drift cannot make seeded timelines return 500s.docs/debugging-and-qa.md.Why
Mobile slowness reports only reproduce at scale. This makes a big, reusable fixture one command away, with deterministic
--seedruns and append mode for jumbo threads (e.g.--projects 1 --threads 8 --events 60000).Notes
~/.bb.--resetalso removesauth.jsonso the daemon re-enrolls under the persisted host id.🤖 Generated with Claude Code