Query Console: what to review โ
| Field | Value |
|---|---|
| Branch | feat/admin-and-merchant-portals, draft PR #976: feat(admin): portal navigation, financials and monitoring, plus the agent-process work behind them, not merged |
| Project | README.md |
| Issue | #958: feat(admin/analytics): promote Query Console to its own surface and teach it to query Firestore |
| Servers you need | Admin portal on http://localhost:3001 (npm run admin:dev). Admin Storybook on http://localhost:6008 (npm run storybook:admin) |
| Local origins | Required. The console reads as BROKEN without them. See the warning below |
Why does this project say "the console reads as broken"? โ
The Firestore endpoints exist only on this branch, so the deployed analytics-api answers 404 and the Firestore tab shows a failure. โ
.env.localpointsANALYTICS_API_ORIGINat Cloud Run, which runs whatever last merged todev./analytics/admin/firestore-queryis not there.- This is not a defect in the console. Point the origin at a local analytics-api and it works: a real job, 403 rows, cache miss.
- Restart the admin dev server after the
.env.localorigin change lands. Tracked as#1002: fix(dev-env): local dev proxies venues and analytics to Cloud Run, so branch backend changes are invisible locally. - This exact confusion already happened once, mid-session, on your own stack. It is written up in
implementation.md.
What changed here? โ
The Query Console became its own Analytics surface, out from under BigQuery. โ
- It was a tab buried under Analytics > BigQuery, which made it read as a BigQuery feature rather than the place you go to ask the data a question.
- Now
/admin/analytics/query-console, its own sidebar entry. The oldbigquery/consolepath redirects.
Firestore got a structured query builder, not SQL. โ
- Firestore has no query language, so this is collection plus filters plus order plus limit, executed server side against a field allowlist.
- The rejected alternative was the Firebase Stream Firestore to BigQuery extension. It would have given real SQL with zero new UI, and it was turned down because it keeps a second live copy of user documents outside Firestore.
The surface was found mechanically compliant and functionally undiscoverable, and that second half is now fixed. โ
- Your own example made the case: "how many lanterns are lit at venue X" is answerable, via
venues.activeLanternCount, and nothing led anyone to it. - Slice 6 added a Firestore rail, descriptions inside the pickers, the refused collections with reasons, saved inquiries, and a reference doc linked first in the tab strip.
Two filed bugs were fixed, and the fix run turned up four more. โ
- The console was shipping every document id, which no allowlist reviewed.
merchantProfileswas enumerating Firebase Auth UIDs 500 rows per request. - A recovered schema fetch still showed the error, and Cmd+Enter bypassed the disabled Run button. The held-shortcut storm measured 65 to 87 concurrent queries.
The Assistant stopped guessing at BigQuery, and stopped sending Auth UIDs to Claude. โ
- Asked for a query it used to produce
your-project.firestore_export.offers, a generic placeholder project plus the default dataset name of an extension this project has never installed. Neither exists. - It now reads the real schema. Reading the schema decides what EXISTS, never what is ALLOWED.
- The code review found it was shipping Firebase Auth UIDs to an external processor. Fixed, along with five more dead field names.
Where do I look, and what am I looking for? โ
The console http://localhost:3001/admin/analytics/query-console โ
- Needs the local origins.
- Its own sidebar entry under Analytics, named Query Console, between BigQuery and Configurations.
/admin/analytics/bigquery/consoleshould redirect here.- The tab strip should read: Reference Docs (external, first), then BigQuery, then Firestore.
- The reference doc itself is
docs/engineering/analytics/query-console.md, new on this branch. With the docs site running (npm run docs:dev) it is athttp://localhost:5174/engineering/analytics/query-console. - The BigQuery tab should no longer offer a Console tab of its own.
The Firestore tab, and its rail โ
- Two rail tabs: What you can ask and Saved.
- "What you can ask" is literally questions in the words someone would say out loud, and clicking one fills the builder in. It is not a field reference. Judge it on that: can you find
venues.activeLanternCountby asking "how many lanterns are lit here" rather than by knowing the field name? - "Not readable, on purpose" should list the refused collections with a reason each. An absent collection and a missing feature are indistinguishable from the outside, so a user hunting for
lanternswould otherwise read a deliberate privacy decision as an oversight. - The search should force matching groups open so a match cannot hide behind a collapsed chevron.
- Per-field explanations belong in the pickers, where the choice is made.
The rail in Storybook http://localhost:6008 โ
The BigQuery tab โ
analytics.eventsis day partitioned and REJECTS any query without a filter ontimestamp. The failure message names partition elimination rather than a missing filter, so the console guidance has to say so. Check that it does.- Cmd+Enter should not run a query while the Run button is disabled, and holding it should run one query, not 65.
- A schema fetch that fails and then recovers should stop showing the error.
The Assistant floating chat in the bottom corner of any admin page, for example http://localhost:3001/admin โ
- Needs the local origins, since the assistant-api tools reach BigQuery through the schema reader.
- Ask it for a BigQuery query and check it names a real dataset. The nine datasets in
lantern-app-devareanalytics,billing_attrib,billing_export_gcp,billing_exports_gcp,billing_marts,billing_norm,billing_raw,logs,ops. There is nofirestore_exportand no Firestore export of any kind. analyticsholds five objects:ad_delivery_daily,event_counts_daily,events, plus therecent_system_eventsandrecent_user_eventsviews.
What is deliberately NOT done? โ
The launch plan row for this work does not exist yet. โ
PROTOTYPE.mdPhase 2 (Build), "Analytics hardening". The row text is drafted and waiting on you or a builder session, because the PM lane guard blocks a write to the launch pages. (WasALPHA.mdPhase 1 before the 2026-08-31 timeline shift.)
There is no submission or moderation state in BigQuery at all. โ
- Offer-shaped events in the last 90 days are
sponsored_offer_filled(155),offer_claimed(5),offer_redeemed(2). The question that started this project cannot be answered there, which is why the Firestore side exists.
The Firestore field allowlist is the exact decision #874: design(privacy): one data-classification and egress policy, before search, the assistant, and the apps platform each invent their own says should not be invented per surface. โ
- It was settled here anyway, because the console needed one. A gate was added so an allowlisted field that nothing writes fails the build, but the policy question is still open.
What open findings touch this project? โ
Cross-referenced from ../merchant-and-venue-dashboards/branch-review-0828.md. Three of them, and one is a data-loss path.
A saved query loaded on one console tab is overwritten by a save on the other. ConsolePanel.jsx:187 โ
ConsolePanelstays mounted across the BigQuery and Firestore tab switch, andloadedQueryis neither cleared nor checked for source.- Load a SQL query, switch to the Firestore tab, build a query, save: the dialog treats it as an update and rewrites the SQL query's document. The original saved query is gone. The reverse direction is the same bug.
- Related: the BigQuery rail is passed
saved.queriesunfiltered, so a Firestore inquiry appears there and loading it blanks the SQL editor. - Open, not fixed. It is the least likely to be hit by accident and the most expensive when it is.
A digit-only filter value is coerced to a Number. useFirestoreConsoleState.js:245 โ
merchantId == 1234567890queries a number against a string field and renders a confident empty grid. Open, not fixed.- If a Firestore filter returns nothing and you expected rows, try a value that is not all digits before believing the result.
A saved inquiry loaded before the schema settles has its limit silently replaced. useFirestoreConsoleState.js:69 โ
- The schema effect unconditionally resets the limit. Open, low priority.
The allowlist linter's report asserts checks it did not run. lint.firestore-allowlist.js:372 โ
- The "Verified" lines and the allowlist count print unconditionally, including where a source could not be parsed. The exit code is right; the report is not. Open, low priority.
What was fixed in the allowlist gate on 2026-08-30? โ
The gate could stop looking and still print PASS, so everything downstream was trusted on a check that was not running. โ
- Found by
/code-review maxonfeat/admin-and-merchant-portalsas finding 18, againsttooling/scripts/lint.firestore-allowlist.js:116. - Fixed rather than filed under AGENTS.md rule 18: a guard silently doing nothing degrades everything built on it while it waits.
How it was reproduced. โ
- The two allowlist files were mirrored into a scratchpad repo with symlinks, so nothing in
services/api/was touched on the shared checkout, and the mirror reproduced the shipped run exactly: 42 fields across 8 collections, PASS. - Reindenting the policy body two spaces with the
exportline untouched: the console source went from 23 fields to 0, the totals fell to 19 across 4, and the run still printed PASS with exit 0. - The sharpest version of the same reproduction: a genuinely dead field (
merchantProfiles.lanternGlowIndex, nothing in the repo writes it) FAILS with exit 1 on the shipped policy, and the SAME dead field with the body reindented PASSES with exit 0. A real violation was hidden by a whitespace change. - Second copy of the same defect, not in the review: the assistant parser required a whole array on ONE line, so wrapping the
venueslist prettier-style dropped that collection entirely, 42 fields to 35, PASS.
What the existing defence missed. โ
- The file already refused to go green on a null parse, with the words "went blind rather than green". That defence covered exactly one shape: the top-level object going missing.
- It never asked whether the object it FOUND produced anything. A parser that located the export and then matched none of its contents returned an empty map, which is not null, so it fell straight through the guard and cost zero iterations.
- Zero iterations has no output of its own. "Nothing to check" and "nothing wrong" print the same line, which is the same ambiguity the CONTROL check exists for one layer out.
What the fix does, in two halves. โ
- The parsers no longer read formatting. Both are brace-matched tokenizers now, so indentation, line breaks, wrapping, comments, and braces or quotes inside a description are all irrelevant. This is the half that keeps the gate from firing on a legitimate reformat.
- The existing guard was widened, not duplicated.
blindnessReason()treats null, zero collections, and zero fields as the same event and routes all three into the one error the file already had. An object that never closes returns null rather than a partial list. - The report now counts per source. A total hides a source that contributed nothing, which is exactly the state the gate went green in; a blind source now prints
BLINDon its own line. This does not close the separate open finding above about the "Verified" lines printing unconditionally.
Both proofs, run in the mirror against the shipped script and the fixed one. โ
| Case | Shipped | Fixed |
|---|---|---|
| Policy as it ships | 42 fields, PASS, exit 0 | 42 fields, PASS, exit 0 |
| Policy body reindented two spaces | 19 fields, PASS, exit 0 | 42 fields, PASS, exit 0 |
fields: renamed to columns: | 0 console fields, PASS, exit 0 | BLIND, FAIL, exit 1 |
| The export renamed | FAIL, exit 1 | FAIL, exit 1 |
| Assistant array wrapped across lines | 35 fields, PASS, exit 0 | 42 fields, PASS, exit 0 |
| A real dead field, policy as it ships | FAIL, exit 1 | FAIL, exit 1 |
| The same dead field, body reindented | PASS, exit 0 | FAIL, exit 1 |
- Row 2 is the MEANING proof: a legitimate reformat does not fail, it just keeps being read.
- Row 3 is the blindness proof: a restructure the parser genuinely cannot follow now fails loudly.
- Rows 6 and 7 are the real-violation proof: the gate still catches what it was built for, and no longer loses it to whitespace.
The three ratchet checks, from the linter-organization rule. โ
- DIRECTION: can this hide a violation? No. Every changed path can only ADD a failure. The parsers return a superset of what they returned before (verified identical on the real files: 42 fields, 8 collections, same names), an unclosed object now returns null instead of a partial list, and
blindnessReasonadds three error conditions and removes none. Nothing that failed before passes now. - MEANING: would it fire on correct behaviour? The risk here was a parser made stricter, which starts failing on a legitimate reformat and then gets switched off, which this repo has already had happen with a guard that cried wolf. The fix went the other way: it is robust to formatting rather than demanding one. The one new assertion is deliberately SOURCE-level, never per collection, because a single collection exposing only its document id legitimately has no fields; an entire allowlist naming nothing is not a state the console can be in, since it would refuse every collection it is built to serve.
- CONTROL: does the gate still fire? Not strictly required, since this TIGHTENS rather than loosens, but a fix to a blind gate is the last place to take a green on trust. Two deliberate violations were planted outside the change: a dead field, which fails on the shipped and the fixed script alike, and the same dead field under a reformat, which only the fixed script catches. Both recorded in the table above with exit codes.
Regression cover. โ
tooling/scripts/__tests__/lint.firestore-allowlist.test.js, 17 tests, run bynpm run test:tooling.- Every formatting case in it returns
{}or drops a collection under the shipped parsers, which was checked directly rather than assumed. A test that passes on the broken code proves nothing.