Skip to content

Commit 287a282

Browse files
ahmetrendeclaude
andcommitted
rename the run's audit view, and list a foreign table
`qh-panels.jsx` and `qh-admin-audit.jsx` each declared a top-level `AuditView`. The bundle imports each file as a module, so the two stayed in separate scopes and it never showed there; the raw prototype loads them as scripts, where every top-level declaration is a global and the later file wins -- the developer view's Audit tab rendered the admin screen with no props and threw. prototype-globals-collide.test.mjs now fails if two files that load together declare the same top-level name. A relation kind left off the browser's list does not fall back to `table`: it is dropped from the query, so the relation is absent from the tree, from autocomplete and from fleet search. `foreign` was off it, and a foreign table is selected from like any other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 53599fd commit 287a282

4 files changed

Lines changed: 105 additions & 3 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Two files, one top-level name — fine in the bundle, broken in the prototype.
2+
//
3+
// The build imports each source as an ES module, so a top-level `function X`
4+
// is module-scoped and two files may each have their own. The raw prototype
5+
// loads the same files as Babel <script> tags, where every top-level
6+
// declaration is a GLOBAL: the last file loaded wins, and the earlier file's
7+
// own call sites silently get the other one.
8+
//
9+
// That is how `AuditView` behaved. `qh-panels.jsx` had one for a single run's
10+
// trail and `qh-admin-audit.jsx` one for the admin screen; in the prototype the
11+
// developer view's Audit tab rendered the ADMIN component with no `st` prop and
12+
// threw `Cannot read properties of undefined (reading 'pushToast')`. The build
13+
// showed nothing, which is exactly why this is a test and not a habit.
14+
//
15+
// The mirror image of portal-globals.test.mjs: there the prototype was right
16+
// and the build broken; here the build is right and the prototype broken. Both
17+
// come from the same asymmetry, so both need a check nobody has to remember.
18+
import test from 'node:test';
19+
import assert from 'node:assert/strict';
20+
import { readFileSync, readdirSync } from 'node:fs';
21+
import { fileURLToPath } from 'node:url';
22+
import { dirname, resolve } from 'node:path';
23+
24+
const HERE = dirname(fileURLToPath(import.meta.url));
25+
const SRC = resolve(HERE, '..', 'src');
26+
27+
// `qh-api.jsx` and `qh-api-mock.jsx` are ALTERNATIVES — the prototype loads one
28+
// or the other, never both — so they are meant to define the same names.
29+
const ALTERNATIVES = new Set(['qh-api.jsx', 'qh-api-mock.jsx']);
30+
31+
// Known and reported to design, not blessed: `qhAgo` exists twice with
32+
// DIFFERENT signatures (qh-panels takes an epoch number, qh-admin-data an ISO
33+
// string). It is their file and their call, so it is named here rather than
34+
// renamed unilaterally — and naming it keeps this test able to fail for
35+
// anything new. See CODE_TO_DESIGN_BRIEF.md, 2026-09-15.
36+
const KNOWN = new Set(['qhAgo']);
37+
38+
function topLevelNames(src) {
39+
const out = new Set();
40+
for (const m of src.matchAll(/^(?:function|class|const)\s+([A-Za-z_$][\w$]*)/gm)) {
41+
out.add(m[1]);
42+
}
43+
return out;
44+
}
45+
46+
test('no top-level name is declared by two files that load together', () => {
47+
const owners = new Map();
48+
const clashes = [];
49+
for (const f of readdirSync(SRC).filter(f => f.endsWith('.jsx')).sort()) {
50+
for (const name of topLevelNames(readFileSync(resolve(SRC, f), 'utf8'))) {
51+
const prev = owners.get(name);
52+
if (prev === undefined) { owners.set(name, f); continue; }
53+
if (ALTERNATIVES.has(prev) && ALTERNATIVES.has(f)) continue;
54+
if (KNOWN.has(name)) continue;
55+
clashes.push(`${name}: ${prev} and ${f}`);
56+
}
57+
}
58+
assert.deepEqual(clashes, [],
59+
'in the prototype the later file wins and the earlier one gets a stranger');
60+
});
61+
62+
test('the run trail and the admin screen are not both AuditView', () => {
63+
const panels = readFileSync(resolve(SRC, 'qh-panels.jsx'), 'utf8');
64+
assert.match(panels, /function RunAuditView\b/,
65+
'a run\'s trail is RunAuditView; AuditView is the admin screen');
66+
assert.doesNotMatch(panels, /function AuditView\b/);
67+
assert.match(panels, /<RunAuditView\b/, 'and it is what the tab renders');
68+
});

‎QueryHubWeb/qh-panels.jsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,7 @@ function ResultsPanel({ tab, setTab, result, messages, audit, status, runMs, onE
12951295
{tab === 'results' && <ResultsView key={'st' + stN} result={result} status={status} onToast={onToast} colMeta={colMeta} unmasked={unmasked} />}
12961296
{tab === 'plan' && <PlanView plan={plan} />}
12971297
{tab === 'messages' && <MessagesView messages={messages} />}
1298-
{tab === 'audit' && <AuditView audit={audit} />}
1298+
{tab === 'audit' && <RunAuditView audit={audit} />}
12991299
</div>
13001300
</div>
13011301
);
@@ -1854,7 +1854,16 @@ function MessagesView({ messages }) {
18541854
</div>;
18551855
}
18561856

1857-
function AuditView({ audit }) {
1857+
// A RUN's audit trail — the events of the one request in this results panel.
1858+
// NOT the admin Audit screen, which `qh-admin-audit.jsx` also called
1859+
// `AuditView`. In the raw prototype every top-level function is a global and
1860+
// the later script wins, so this component rendered the ADMIN screen with no
1861+
// `st` prop: the developer view's Audit tab threw `Cannot read properties of
1862+
// undefined (reading 'pushToast')` and took the page to the error boundary.
1863+
// The bundle never showed it — main.jsx imports each file as a module, so the
1864+
// two stayed in separate scopes — which is the whole reason to rename rather
1865+
// than rely on that.
1866+
function RunAuditView({ audit }) {
18581867
if (!audit.length) return <div className="qh-empty"><div>Audit entries appear after you submit a query.</div></div>;
18591868
return <div className="qh-audit">
18601869
{audit.map((a, i) => (

‎src/queryhub/web/routes_data.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,14 @@ def _catalog_functions(target_id: int, database: str) -> list[dict]:
136136
# Relations the browser lists. Views belong here: a view is as queryable as a
137137
# table, so autocomplete and the tree both want it. What it is NOT is a table,
138138
# and the ref carries `k` so the reader can be told which one it is looking at.
139-
_LISTED_RELKINDS = ("table", "partitioned", "view", "matview")
139+
#
140+
# `foreign` is listed for the same reason -- a foreign table is selected from
141+
# like any other, and a kind left out of this tuple does not fall back to
142+
# "table": it is dropped from the list entirely and the relation becomes
143+
# invisible in the tree, in autocomplete and in fleet search. There are none in
144+
# this fleet today, which is exactly why it had to be decided now rather than
145+
# the day one appears.
146+
_LISTED_RELKINDS = ("table", "partitioned", "view", "matview", "foreign")
140147

141148

142149
def _catalog_table_refs(target_id: int, database: str) -> list[dict]:

‎tests/test_connections_batched_reads.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,24 @@ def test_a_view_is_listed_but_marked_as_one(monkeypatch):
124124
("daily_totals", "matview")]
125125

126126

127+
def test_a_kind_left_off_the_list_disappears_rather_than_defaulting(monkeypatch):
128+
"""The client reads a ref with no `k` as a table. A kind missing from
129+
`_LISTED_RELKINDS` never gets that far: the SQL drops it, so the relation is
130+
absent from the tree, from autocomplete and from search -- silently, and in
131+
the same shape as the bug this list was added to fix."""
132+
rows = [_tbl(1, "a", "public", "orders"),
133+
_tbl(1, "a", "public", "remote_ledger", "foreign"),
134+
_tbl(1, "a", "public", "an_index", "index")]
135+
monkeypatch.setattr(routes_data.db, "fetch_all",
136+
lambda *a, **k: [r for r in rows
137+
if r["relkind"] in routes_data._LISTED_RELKINDS])
138+
got = routes_data._catalog_table_refs_map([(1, "a")])[(1, "a")]
139+
assert [r["n"] for r in got] == ["orders", "remote_ledger"], (
140+
"a foreign table is selected from like any other and must be listed")
141+
assert "foreign" in routes_data._LISTED_RELKINDS
142+
assert "index" not in routes_data._LISTED_RELKINDS
143+
144+
127145
def test_the_single_read_and_the_batched_one_agree_on_shape(monkeypatch):
128146
"""The tree renders from whichever of the two answered. A ref missing `k`
129147
from one of them is a view drawn as a table on that path only."""

0 commit comments

Comments
 (0)