Skip to content

feat(cli): add assistant db status for read-only DB inspection#32606

Merged
dvargasfuertes merged 1 commit into
mainfrom
apollo/assistant-db-status
May 29, 2026
Merged

feat(cli): add assistant db status for read-only DB inspection#32606
dvargasfuertes merged 1 commit into
mainfrom
apollo/assistant-db-status

Conversation

@vellum-apollo-bot
Copy link
Copy Markdown
Contributor

Follow-up to #32583 addressing both Codex review comments. One was a legitimate bug; the other was a misread of React reconciliation, locked in as a regression test instead.

Codex P2 #1 — Legitimate bug, fixed

Avoid auto-pinning below assistant-only history. The scroll coordinator auto-pins to the bottom on conversation switches, so assistant-only histories/onboarding content land just above the viewport and the user sees blank space plus the avatar instead of the latest message.

The merged code applied style={{ minHeight: viewportMinHeight }} to the latest-edge wrapper whenever renderAvatar was present — even with partition.anchorMessage === null (assistant-only history: recovered conversations whose user message was lost, onboarding before the first submit).

useViewportMinHeight reports the scroll container's clientHeight ≈ a full viewport. refactor(web): scroll to bottom on transcript container DOM attach (#32239) snaps the scroll to bottom on conversation switch. With no anchor at the top of the spacer, the bottom-pinned viewport lands on blank space + the avatar — the actual latest assistant message sits one viewport above and is invisible until the user scrolls up.

Fix: gate both minHeight: viewportMinHeight and the flex-1 spacer on partition.anchorMessage. Without an anchor, the avatar renders inline directly below the last history item.

Codex P2 #2 — Misread, locked in as a regression test

Preserve the avatar when the first turn is inserted. React reconciles the following <div>s by index, so the current avatar wrapper is reused as the spacer and ChatAvatar unmounts/remounts, replaying the entrance animation this change is trying to avoid.

Empirically verified that this does not happen. React's reconcileChildrenArray tracks fiber.index (the previous render's position in the children array, including false-skipped slots). When slot 0 transitions from false to <LatestTurnRow>, the existing avatar fiber at fiber.index=2 still matches newIdx=2 in the next render — same <div> type, fiber reused, ChatAvatar is not unmounted.

Locked in with a DOM-lifecycle regression test that:

  1. Renders the transcript with no anchor + a mount-tracking avatar.
  2. Asserts mount=1, unmount=0.
  3. Rerenders with the first user message inserted (no-anchor → anchor).
  4. Asserts mount=1, unmount=0 (no remount).
  5. Rerenders back to no-anchor.
  6. Asserts mount=1, unmount=0 again.

A sanity check (forcing a conversationId change, which re-keys the scroll container) was used to confirm the mount tracker actually detects remounts when they happen (mount=2, unmount=1). The test framework can fail; the assertions are real.

Tests

  • 13 transcript.test.tsx tests pass.
  • 3 new Codex feat: initialize Next.js app in /web directory #1 cases: no-anchor wrapper has no min-height; anchor wrapper has min-height; anchor-without-avatar still has min-height (regression guard for the original viewport-pinning behavior).
  • 1 new Codex feat: add platform terraform for GKE deployment #2 case: the mount-tracker test above.
  • Typecheck clean (tsc --noEmit --skipLibCheck in apps/web).
  • ESLint clean on touched files.
  • 4 pre-existing transcript-message-body.test.tsx failures in the directory-wide run reproduce on main too — caused by mock bleed between test files, not by this PR.

Why this is a follow-up rather than amending the original

#32583 was already merged before Codex's review surfaced. Per the SE-skill pr-lifecycle.md rule on bot reviewers being real reviewers, every comment gets a response — either an iteration or a rationale. This PR is both: iterates on #1, rationalizes #2 with empirical evidence.

First subcommand under a new `assistant db` group dedicated to inspecting
and (in follow-ups) repairing the assistant SQLite database. Designed for
recovery / triage: opens the DB file directly via bun:sqlite in read-only
mode and never goes through the daemon — the surface needs to work
precisely when the database is broken and the daemon won't start.

Reports:
- file path, sizes (.db / .db-wal / .db-shm), owner uid/gid + mode
  (the latter catches UID-isolation regressions on shared volumes)
- SQLite version and key pragmas (journal_mode, synchronous, page_size,
  wal_autocheckpoint, mmap_size)
- total table count + 5 largest tables — by `dbstat` bytes when the
  runtime SQLite has it compiled in, otherwise by row count. Bun's
  bundled SQLite omits dbstat, so the row-count path is what most
  users hit today; we detect at runtime via a try/catch rather than
  parsing PRAGMA compile_options strings (which has historically
  drifted across versions)
- latest applied migration, read from `memory_checkpoints` rows that
  the workspace migration runner writes with value='1' on success
- file mtime + age, for 'is this DB live?' triage

If the DB is missing, exits 1 with a loud error pointing the user at
`assistant backup list` and (preview of PR 3) the conversation
disk-view recovery path.

Naming note: cli-design.md says `get` over `status` as a convention,
but I'm shipping under `status` per the recovery-plan ask. Happy to
rename in a follow-up if we want to enforce the convention here too.

Wired up:
- new directory: `assistant/src/cli/commands/db/` with
  `index.ts`, `status.ts`, `format.ts`, and tests
- transport: `local` — no daemon roundtrip, no IPC
- registered in `program.ts` alphabetically between `credentials`
  and the email-gated commands
- added `db` and `db status` to the gateway risk-registry
  supported-paths list (no override needed — read-only defaults to
  the low-risk baseline)

Tested:
- 7 unit tests covering happy path, missing-DB error, --json shape,
  the 'started' vs '1' migration filter, and the
  no-memory_checkpoints-table fallback
- smoke-tested against the live workspace database

Refs: recovery + monitoring workstream
@vellum-apollo-bot vellum-apollo-bot Bot force-pushed the apollo/assistant-db-status branch from bdb947d to 9d1f33f Compare May 29, 2026 19:49
name: "db",
transport: "local",
description:
"Inspect and repair the assistant SQLite database (read-only by default)",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Inspect and repair the assistant SQLite database (read-only by default)",
"Inspect and repair the assistant SQLite database",

it's not read only by default?

@dvargasfuertes dvargasfuertes merged commit 2f045ed into main May 29, 2026
14 checks passed
@dvargasfuertes dvargasfuertes deleted the apollo/assistant-db-status branch May 29, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant