Skip to content

Implementation โ€‹

What was done on 2026-08-27? โ€‹

Both filed bugs are fixed and verified live, and the run turned up four more. โ€‹

  • Branch feat/query-console-958. Verified on a local stack (admin portal 3011, analytics-api 8092) driven headlessly, because the endpoints exist only on this branch.

What did #970 actually need? โ€‹

The document key now goes through the same review as every field. โ€‹

  • projectDoc shipped id unconditionally, so the one value always present was the one value no allowlist reviewed. The file header promised the opposite.
  • Confirmed against real data before deciding: merchantProfiles document ids are 28 characters and match the Firebase Auth UID shape. venues, offers and featureRequests are 20-character auto-ids. So the leak was real and it was specific.
  • exposeId is now declared per collection beside its fields, defaulting to off. venues, offers and featureRequests keep their key; merchantProfiles does not, because its key is a Firebase Auth UID. (featureFlags was also listed here until it was removed from the allowlist entirely later the same day.)

Projection stops at depth one. โ€‹

  • toCellValue recursed into maps and emitted every nested key, so the default-deny guarantee held only at the top level.
  • Checked the data first: no allowlisted field holds a map today, so the cap regresses nothing. A map is summarized as <map, N keys>, never by key name.
  • venues.source is a scalar today and the import pipeline already writes richer provenance maps beside it, which is why this was worth closing now.

The audit log records what was asked for. โ€‹

  • queryMeta now carries filters: [{field, op}], fields and operators only. A filter value can carry the thing being looked for, so it stays out.

The route-level suite exists. โ€‹

  • 12 tests at services/api/analytics/test/routes/firestoreQuery.test.js, covering the zod layer, the refusal-to-HTTP mapping and the audit log, none of which the service suite reached.

What did #971 actually need? โ€‹

The schema error resets, so a cold start no longer kills the source. โ€‹

  • setSchemaError(null) before each retry. Verified: forced a 503, saw the error, let it recover, and the builder came back with no reload.

A held shortcut runs one query. โ€‹

  • Measured before: holding Cmd+Enter for one second fired 65 requests on one run and 87 on another, against a Run button rendered disabled the whole time. The issue estimated 30.
  • Fixed at both layers: the keydown handler ignores e.repeat and consults isRunning, and the hook takes an in-flight guard plus a last-write-wins ticket so a slow failure cannot clear a fast success.
  • The same guard went on the BigQuery hook, which had the identical gap.
  • Measured after: 1 request.

The Limit box holds a string. โ€‹

  • Number('') is 0, and 0 ?? 50 is 0, so clearing the field wrote a literal 0 under the cursor and then earned a 400. The value is clamped at send time and normalized on blur.

What else was found, and where did it go? โ€‹

FindingState
Four control heights in one filter row, and the stylesheet's declared 38px was not the rendered 42pxFixed, measured live
A non-JSON error body rendered as a full HTML document in the error stripFixed
The status strip toggled role onto an element already mounted, so nothing announced a runFixed, stable role="status" + aria-live + aria-busy
Removing a filter dropped focus to <body>Fixed, focus returns to Add filter
A result row was mouse-only, and it is the only way to read a truncated valueFixed, focusable and answers Enter and Space
The row filter input had no accessible nameFixed
The two sources diverge on failure: Firestore clears the grid, BigQuery keeps itRecorded in design.md, product call
The sidebar nav entry is keyboard-unreachableOut of scope by dispatch, already tracked as #922: the portal's sidebar navigation cannot be reached or activated by keyboard

Why was the console dead on the operator's own stack? โ€‹

The portal proxies to whichever analytics-api ANALYTICS_API_ORIGIN names, and that is the DEPLOYED one. โ€‹

  • apps/admin/vite.config.mjs sends /api/analytics to ANALYTICS_API_ORIGIN, which in .env.local is the Cloud Run service. That build predates these endpoints, because they have never been merged to dev.
  • Express answered 404, and the console pasted the raw HTML 404 page into the error strip. The display half of that is fixed; the configuration half is an environment choice, not a code bug.
  • This is not a stale-process problem. The route is genuinely registered in the local build (introspected the router: 15 routes, including GET /firestore-schema); nothing was pointed at it.
  • A local analytics-api on any port plus ANALYTICS_API_ORIGIN set to it makes the tab work immediately.

How was it verified? โ€‹

13 of 13 checks passed on the running console. โ€‹

  • Routes loaded: /admin (sign-in) and /admin/analytics/query-console, both tabs.
  • The decisive two: merchantProfiles returned rows keyed ["status","createdAt"] with no document key and no id column in the schema; offers returned 9 rows still carrying id.
  • Test suites: 28 service tests, 12 route tests, 164 admin analytics frontend tests, all passing. lint:emdash and lint:stories both pass.

Two harness gotchas worth keeping. โ€‹

  • storageState does not carry a Firebase session. Firebase Auth persists in IndexedDB, which storageState ignores, so a "reuse the session" run signs in again and spends another of the 10-per-15-minutes the admin route allows per IP. launchPersistentContext with a fixed profile directory is what actually works.
  • isSearchable={false} makes react-select render its input outside the viewport. Clicking the input id times out; the visible -control element is the clickable one.

What did the operator ask for during the session, and in what order? โ€‹

Recorded in her words rather than paraphrased, because most of this branch is her redirects rather than the dispatched scope. The task that went out was two filed bugs plus a design verification; this is everything that arrived after.

1. The opening scope: two halves. โ€‹

"It isn't finished. Looking at the firestore query console, it has bugs. I also want to verify the design of it as well."

  • Delivered: #970 and #971 fixed and browser-confirmed, four further bugs found and fixed, and design.md.

2. She hit the dead Firestore tab herself, mid-session. โ€‹

"FYI I am still seeing this error when I try to look at the firestore query console: Could not load the Firestore allowlist: ... Cannot GET /analytics/admin/firestore-schema"

  • Cause: her portal proxies /api/analytics to the DEPLOYED analytics-api, which predates these endpoints. Not a code bug, but the console rendering a raw HTML 404 page into its error strip WAS one, and that is fixed.
  • She chose to work against a local stack rather than repoint hers.

3. Discoverability, raised once the console worked. โ€‹

"I see it working now, thank you! That said, we definitely need some design and functionality considerations. First off, we probably need to create a reference doc for this. Or at bare minimum have some breadcrumbs ( like what is firestore for vs bigquery? ). Also, how would someone know how to even query firestore? For example, how would they know to query how many lanterns are lit at an x venue for example?"

  • Her lanterns example is the sharpest finding of the day. That question IS answerable, through venues.activeLanternCount, and nothing on the surface led anyone to it, because a user would go looking for a lanterns collection that does not exist and never will.
  • Four directions were mocked with the app's real CSS before anything was built.

4. She picked from the mockups. โ€‹

"I think a mix of B and D ( minus that FYI popup on "firestore answers..." ). For B, I think having a similar "what you can ask", but also saved inquires would be helpful. I think we need a simple reference doc and have that at the front of the sub nav line items so it would be something like ( Reference doc | BQ | FS ). Other pages have that for reference."

  • Delivered: the rail, per-option descriptions in the pickers, saved inquiries, and the reference link first in the tab strip matching BigQueryTabs and EventTrackingTabs. No banner.

5. She rejected three parts of that build. โ€‹

"For the what you can ask, why isn't it expand collapse based on collection? Can you look at how BQ is built to please emulate that?"

"For the what you can ask, i literally meant it as what you can ask. If a user clicks, that auto fills."

"There is literally no way you can even save a query"

  • All three were correct. The panel was a field reference wearing her phrase; the interaction was invented rather than copied from SchemaBrowser; and ConsoleToolbar hid Save behind isSql, so the store, the dialog and the panel all worked while the button did not exist.
  • The lesson that generalizes: the design skill says to view the siblings before building, and skipping it is what produced a second interaction model on one page.

6. One question found two live defects. โ€‹

"What are these feature requests and feature flags for?"

  • Answering it by CHECKING rather than by reading the allowlist found that featureRequests named two fields that have never existed, and featureFlags is a collection with zero documents and zero writers anywhere in the repo.
  • Both had been copied from the assistant's allowlist, which planning.md cited as "the working precedent".

What is the pattern across all six? โ€‹

Three separate times, the code asserted something the data contradicted, and nothing failed. โ€‹

  • Status values: descriptions named pending and approved on collections whose real values are open, active, draft, rejected, submitted and pending_setup.
  • Field names: createdAt and issueNumber on a collection that emits submittedAt and githubIssueNumber.
  • A whole collection: featureFlags, allowlisted and advertised, backed by nothing.

None of them errored, which is why they survived. โ€‹

  • A dead field projects to nothing, so the column is merely empty. A dead collection returns zero rows, so it merely looks quiet. An allowlist is not self-verifying, and each later reader treated it as ground truth because it was already there.
  • The guard that fits is the one that runs in CI: firestoreAllowlist.drift.test.js stops the two copies diverging, which is the mechanism that carried the bad names across. It cannot assert a field exists in live data, and that limit is stated in the file.
  • The habit that actually caught all three: read the data before writing anything that names it. Half the starter questions drafted from field names alone would have painted empty tables.

Slice 5: grounding the Assistant (2026-08-27) โ€‹

The Assistant could not read a schema, so "never invent" did not cover the thing it invented. โ€‹

  • Its system prompt contained the word BigQuery zero times, and none of its six tools could read a schema. The never-invent guardrail named file paths, function names, Firestore field names and policy details, and stopped there.
  • Asked for a query on 2026-08-24 it produced your-project.firestore_export.offers: a placeholder project plus the default dataset name of a Firebase extension this project has never installed.

The fix is to let it READ the schema, not to extend the list it is told to trust. โ€‹

  • Slice 5 as planned said to extend the guardrail to cover BigQuery names. Today proved a list is exactly what fails silently: the Firestore allowlist beside it was advertising a collection with no documents and two fields that never existed.
  • So get_bq_schema returns real dataset, table and column names with their types, and the prompt tells the model to call it before writing any SQL.

Reading the real schema decides what EXISTS, never what is ALLOWED. โ€‹

  • BQ_ALLOWED_DATASETS is analytics alone. The project has nine datasets: six billing_* carrying vendor invoice detail, plus logs and ops. None earn a place by default.
  • Default deny is structural rather than by omission. The tool never calls getDatasets(). It opens only the datasets it names, so an enumeration bug cannot show everything and a dataset created tomorrow cannot become visible by existing. A test asserts getDatasets is never called, and another adds a new dataset to the fixture warehouse and asserts it stays invisible.
  • Schema only, never data. Metadata calls create no job and scan no bytes, so there is no code path to a row. Tests assert no rows, sample, preview or numRows key can appear.
  • A refusal says WHY, and distinguishes out-of-scope from no-such-table. A silent empty result reads as "nothing there", which is precisely how the dead Firestore collection hid for months.
  • The scoping is provisional and the comment says so, deferring to #874, the open policy issue for one data-classification and egress rule.

One bug in this work was found only by running it against real BigQuery. โ€‹

  • The client reports the LITERAL string until it has resolved one, so the first working version put that placeholder into the model's context.
  • That is the same failure class the tool exists to prevent: a fake token that reads as a name. Null reads as unknown; a placeholder does not. Pinned by two tests.
  • The mock proved the shape and the live run proved the names. Neither alone was enough, which is the fourth instance today of the same lesson.

A rule-18 find in passing. โ€‹

  • /assistant/feature-requests is user-facing and is mounted ABOVE the admin-gated /assistant chain on purpose, because Express matches mounts in registration order. Reordering them would put requireAdmin in front of a user endpoint and 403 every app user's feature request, and it would look like a broken form rather than a routing change.
  • That was commented thinly. It now says what breaks and why.

What was verified โ€‹

  • Against real BigQuery: 5 real tables in analytics, events correctly reporting partitionedOn=timestamp, none of the other eight datasets appearing anywhere in the output, no row-data keys, no placeholder tokens, and both refusal messages reading correctly.
  • 132 assistant tests, 155 analytics-api, 185 admin. lint:emdash, lint:stories and openapi-sync all pass.

What did the code review find? (2026-08-27) โ€‹

The reviewer used the day's own method, checking the allowlist against the writers, and it found more of the same class.

The assistant was shipping Firebase Auth UIDs to an external processor. โ€‹

  • projectDoc in tools.js built { id } with no per-collection decision, so query_firestore on merchantProfiles returned Auth UIDs, 25 documents at a time, into the Claude API AND the stored conversation log.
  • This is #970 one file over with a worse audience: the console kept those ids in-house. Pre-existing on dev.
  • Fixed with the same exposeId ruling, and the drift test now compares the KEY decision, not only field names. It could not see this divergence at all before, which is the more useful finding: a guard that checks half a decision reports green on the other half.

Five more allowlisted field names existed on zero documents, in BOTH lists. โ€‹

AllowlistedLive presenceReal field
venues.city0 / 300addressCity (112 / 300)
venues.state0 / 300addressState (104 / 300)
venues.tier0 / 300not a venue concept
offers.audience0 / 9targetAudience (9 / 9)
offers.startsAt0 / 9only inside the nested liveEvent map
  • Both copies were wrong IDENTICALLY, which is exactly why the drift test stayed green. Agreement is not correctness, and that limit is now stated in the test file.
  • Verified live after the fix: addressCity carries values on 22 of 50 venue rows and targetAudience on 6 of 6 offers, where the old names returned nothing at all.

The #971 repeat guard was too wide. โ€‹

  • if (e.repeat) return sat above the key branches, so it cancelled EVERY held modifier combination on the page, including undo and redo in the SQL editor. Holding undo in a query editor is an ordinary thing to do.
  • Narrowed to the Enter branch. The hook's in-flight ref stops the duplicate requests either way, so scoping it costs nothing.

Smaller, all confirmed against the writers. โ€‹

  • offers.status omitted pending_review, which is the moderation queue its own description points at, and it is the ONLY path to active. Added, plus a starter question that asks for it.
  • venues.status is written only by the OSM import, so filtering on it silently omits every hand-added venue. Said so in the description rather than leaving it as a trap.
  • venues.updatedAt is written only by a manual edit, so ordering by it returns hand-edited venues only. Same treatment.
  • The DocumentReference check duck-typed on path plus firestore, so a plain map carrying those two keys would have escaped the map summary. Now checked by class, with a test that a map shaped like a reference is still summarized.

Filed rather than fixed here โ€‹

  • #977: no rate limit on /analytics/admin. The #971 fix closed the incident in two React hooks, but a second tab or any non-browser caller with an admin token reproduces 500-row reads without limit. Bigger than this branch.
  • An eslint no-extra-semi error in services/api/assistant/src/services/githubConfig.js, pre-existing since July and present on dev, in a file this branch never touched.

The allowlist reality gate (2026-08-27) โ€‹

A check that fails when an allowlisted name is not written by anything in the repo. โ€‹

  • tooling/scripts/lint.firestore-allowlist.js, wired as npm run lint:allowlist and as "Firestore Allowlist Reality" in the lint scope of the validate orchestrator.
  • Guidance for this class already existed and prevented none of it, which is why this is a gate. The same shape one layer out, a document disagreeing with its own templates, also kept recurring until a gate was built and then stopped.

It crosses the data boundary, which the drift test beside it never does. โ€‹

  • firestoreAllowlist.drift.test.js compares the two allowlists to EACH OTHER. That measures agreement and can never measure correctness: five of the seven dead names were wrong identically in both copies, so it stayed green through all of them.
  • This one compares each name against the code that EMITS it. That is the only crossing that answers the question.

A collection with zero writers is an ERROR, never an empty result. โ€‹

  • featureFlags survived because zero rows reads as quiet rather than wrong. That distinction is the point of the gate, so a writerless collection fails the build outright.

It reports what it checked, and what it did not. โ€‹

  • Every run prints three verified properties and three unchecked ones. A green from a partial check otherwise reads as a green on the whole decision, which is exactly the half-a-decision gap that let the document-key divergence through the drift test.

Measured against the seven, because a check nobody tested is the problem it is fixing. โ€‹

  • Against the PRE-FIX allowlist it reports 6 of the 8 known-bad entries and 0 false positives on the 42 good ones. Two hard failures (featureFlags, offers.audience), four inconclusive (venues.city, venues.state, venues.tier, featureRequests.createdAt), two missed.
  • offers.startsAt is missed because it IS written, nested inside the liveEvent map. Separating a nested key from a top-level one needs a parser.
  • featureRequests.issueNumber is missed because the file that writes that collection also builds a Discord payload carrying a real issueNumber: key. File granularity cannot separate two objects in one file.

A more precise version was built, measured, and rejected. โ€‹

  • Scoping the field search to the text around each write call caught issueNumber, and also failed on venues.name, venues.addressCity, offers.title and offers.targetAudience, every one of which is genuinely written.
  • A gate that fails on a real field gets switched off, and then it catches nothing. File granularity is the deliberate choice, and the rejected approach is recorded in the script so it is not re-attempted blind.

Building it caught a defect in this branch's own code. โ€‹

  • The admin-zones gate flagged react-hooks/set-state-in-effect on the setSchemaError(null) the #971 fix added at the top of the effect.
  • Moved to clear on SUCCESS instead, which is both lint-clean and better behaviour: the error stays visible while a retry is in flight rather than blinking away and back. Re-verified live, the 503 still shows and the recovery still clears with no reload.

Where the guidance half lives โ€‹

  • The verified-done skill owns "before you believe a green". This script is the mechanical half for one specific list, and deliberately does not restate it.

Built with VitePress