id-375 research — fixCore

fixCore

Summary

Both defects are confirmed and worse than reported. (A) detectIsTyped overclaims on BOTH branches of strategy 1: empirical probing against real supabase-js 2.105.4 shows an untyped client’s .from('t') return type is PostgrestQueryBuilder<any, any, any, "t", unknown> — the table-name literal is echoed as a generic even with no Database generic, so branch 1-a (includes(table)) fires true for untyped clients, and branch 1-b fires for any inferred structural builder. Strategy 2 also has a latent bug (clientExpr.getType().getSymbol() resolves to the SupabaseClient interface, never the variable declaration), and the fixture stub masks all of this by dropping the table-name echo. Real-repo spot checks show scripts/eval-classification.ts:228 (client explicitly SupabaseClient<any,...>) and e2e/global-teardown.ts:65/94 (bare SupabaseClient) reported as exact/isTyped: true today. The validated replacement: inspect the .from() return type’s type arguments for a non-any Relation carrying a concrete Row property (8/8 probe cases pass), plus a one-line strategy-2 symbol fix. (B) All 11 result-set queries cap first-come in discovery order, violating PRODUCT inv 14’s spatial-coverage preference; type-evolution and reexport-chain additionally stop enumerating at the limit, undercounting totalEstimated. Design: a new tools/ast-dataflow/truncate.ts exporting truncateSpatial<R extends BaseResult>(allRows, limit) — stable sort by (file, line, column), round-robin-by-file selection when over limit, re-sort of the picked set — integrated via a 6-line mechanical diff per query; flow-trace and reexport-chain are exempt (rows are path/chain hops, not a coverage set).

Findings

A) detectIsTyped overclaim — evidence

The heuristic today

tools/ast-dataflow/queries/supabase-shared.ts:28-77. Strategy 1 (lines 33-43): true if returnTypeText.includes(table) (1-a) OR text has { and lacks unknown/Record<string, unknown> (1-b). Strategy 2 (lines 48-74): client binding’s initializer is a call with explicit type args. Consumed at queries/column-reads.ts:180-181 (confidence = isTyped ? 'exact' : 'indirect') and queries/column-writes.ts:218 → inspectWriteArg(..., isTyped) (column-writes.ts:105-172, exact iff isTyped + literal key). A third near-copy for .rpc() lives at column-reads.ts:259-286.

Empirical probe (real supabase-js 2.105.4, scratchpad probe run with the repo’s ts-morph)

Probe files: /private/tmp/claude-501/-Users-liamj-Documents-development-canonical/90a83277-e0e3-4c0e-bf5b-0d2d569854ae/scratchpad/probe/{typed.ts,tsconfig.json,run-probe.ts,run-probe2.ts,run-probe3.ts}.

from()’s signature (node_modules/@supabase/postgrest-js/dist/index.d.mts:3629) is from<TableName extends string & keyof Schema['Tables'], Table extends Schema['Tables'][TableName]>(relation: TableName): PostgrestQueryBuilder<ClientOptions, Schema, Table, TableName> — the table-name literal is captured as a generic even when Schema = any. Probe output (current heuristic verdicts):

Case	Client	returnTypeText (abridged)	current verdict	correct
typed createClient<Database>()	SupabaseClient<Database,...>	PostgrestQueryBuilder<{PostgrestVersion:"12"}, {Tables:{bid_questions:{Row:{...}}...}}, {Row:{...}}, "bid_questions", []>	true (1-a)	true
untyped createClient()	SupabaseClient<any,...>	PostgrestQueryBuilder<any, any, any, "bid_questions", unknown>	true (1-a)	false
untyped, other table	same	...<any, any, any, "other_table", unknown>	true (1-a)	false
structural interface wrapper (FakeClient)	FakeClient	FakeBuilder (named, no {)	false	false
typed via SupabaseClient<Database> param	—	full typed builder	true (1-a)	true
bare SupabaseClient param	SupabaseClient<any,...>	...<any, any, any, "bid_questions", unknown>	true (1-a)	false
hand-rolled inferred builder	__object	{ select(_cols: string): { eq: ... } }	true (1-b)	false
typed via await on typed factory	—	full typed builder	true (1-a)	true

So the defect is not just branch 1-b: branch 1-a is the dominant false-positive path on the real corpus (the task’s stated 1-b case only fires for inferred structural builders, since named wrapper types print their name, not {).

Fixture stub masks the defect

__tests__/fixtures/07-column-reads/supabase-stub.ts:5-26 (identical in 08-column-writes/) declares from<T extends string>(table: T): QueryBuilder<DB extends {...} ? R : Record<string, unknown>> — QueryBuilder<Row> drops the table-name generic, so the untyped fixture’s return text is QueryBuilder<Record<string, unknown>> (1-a can’t fire, 1-b excluded by Record<string, unknown>). The fixture suite therefore passes while the real-repo behaviour is wrong.

Strategy 2 is dead code (latent bug)

supabase-shared.ts:54 uses clientExpr.getType().getSymbol() — for const sb = createClient<Database>(...) this resolves to the SupabaseClient interface declaration, never a VariableDeclaration, so the type-args check never executes. Verified by probe: my re-implementation with the identifier symbol (clientExpr.getSymbol()) returns true for typed-client.ts:20/25 and false for untyped-client.ts:10/15 (run-probe3 output), while the current lookup returns nothing. Today’s typed fixtures pass only via overclaiming branch 1-b. The rpc copy at column-reads.ts:266 has the same bug.

Real-repo spot checks (CLI: bun run ast-dataflow column-reads --table source_documents --column id --json)

Every returned row today is "confidence":"exact","isTyped":true. Confirmed false positives among them:

• scripts/eval-classification.ts:228 — client is SupabaseAny = SupabaseClient<any, any, any, any, any> (line 101; the file’s own comment at 96-98 says “createClient without database generics”). Should be indirect.
• e2e/global-teardown.ts:65,94 — client from e2e/fixtures/supabase.ts:9 typed bare SupabaseClient. Should be indirect. Confirmed true positives that must stay exact: lib/dashboard.ts:487 (param SupabaseClient<Database>, dashboard.ts:273/355), lib/topic-inference.ts:148 (param SupabaseClient<Database>, line 143). App-route clients are typed: lib/supabase/client.ts:21 returns SupabaseClient<Database>; lib/supabase/server.ts:21 (createSupabaseServerClient<Database>) and :61 (createSupabaseClient<Database>). Note lib/supabase/safe.ts sb() (safe.ts:91-102) is not a client factory — it wraps query thenables (PostgrestLike<T>); typing comes from the factories above, and the awaited-factory pattern (const supabase = await createClient()) is probe case 8: resolvable only via return-type inspection, never via strategy 2.

How isTyped feeds output

Rows expose both isTyped and confidence (types.ts:145 ColumnReadResult, types.ts:182 ColumnWriteResult). PRODUCT inv 15 (PRODUCT.md:223-229) defines exact as “the type checker resolved the symbol” — the current behaviour emits false exact rows, breaching it. Inv 24 (PRODUCT.md:287) forbids false negatives in the exact tier only; the fix demotes false-exact rows to indirect without dropping any row, so inv 24 is unaffected.

B) Spatial-coverage truncation — survey

PRODUCT inv 14 (PRODUCT.md:216-221): cap (default 200), truncated/total_estimated, “Truncation prefers spatial coverage — distinct files come first, multiple hits per file thin last.” Envelope: types.ts:52-57 (totalEstimated? present only when truncated). TECH.md test rows: line 592 (limit:1 truncation) and 786 (truncation.test.ts).

Per-query capping pattern (all first-come in project.getSourceFiles() discovery order unless noted):

Query	Cap sites	Pattern	Spatial applies?
callers.ts	84-85	totalEstimated++; if (rows.length >= limit) continue	yes
references.ts	171-172	same	yes
importers.ts	256-257, 313-314 (two passes, one rows array)	same	yes
column-reads.ts	231-232, 288-289	same (lineCol computed only for kept rows)	yes
column-writes.ts	236-237	same	yes
enum-uses.ts	122-123, 146-147, 214-215	same	yes
dead-exports.ts	227-228	same	yes
string-literal-uses.ts	157-158, 190-191	same	yes
type-evolution.ts	193/199/216 (collector breaks at limit — stops enumerating, undercounts totalEstimated), 364-365, 395-396	mixed	yes
type-drift-detect.ts	752-754 (limit 500); classification-major sort at 789	same	yes, but classification order is the primary key
reexport-chain.ts	128 (BFS stops at rows.length < limit — totalEstimated is a lower bound), 338-339 slice, 355-356	collect-then-slice	no — rows are chain hops with distance; spatial reorder would break barrel→importer contiguity
flow-trace.ts	~20 sites, state.rows.length < state.limit	streaming during DFS	no — rows are ordered hops of a trace path (perf tests assert hop count/order, performance.test.ts:56,100)

Ordering sensitivity: no query sorts rows spatially today; output order = ts-morph file discovery order (nondeterministic across tsconfig glob changes). Tests are almost all order-agnostic (they .sort() before asserting — callers.test.ts:33,78, importers.test.ts:26,199, rename-sweep-skill.test.ts:156); only single-row assertions use results[0] (callers.test.ts:101, type-drift-detect.test.ts:338). type-drift-detect’s Markdown report (cli.ts renderMarkdownReport) depends on classification-major order.

Recommendations

A) detectIsTyped replacement (supabase-shared.ts:28-77)

What constitutes proof of a Database-generic client: the .from('table') call’s return type is a generic instantiation whose type arguments include a Relation with a concrete Row shape. Typed clients instantiate PostgrestQueryBuilder<ClientOptions, Schema, Relation, TableName, ...> with Relation = { Row; Insert; Update; Relationships }; untyped clients instantiate every schema-derived argument as any. Text matching is abandoned entirely — the table-name echo makes it unsound.

Exact replacement (validated 8/8 on the probe corpus and on the existing fixture stubs):

export function detectIsTyped(fromCallExpr: CallExpression, table: string): boolean {
  // Strategy 1: the .from() return type's type arguments must include a
  // non-any Relation carrying a concrete `Row` shape. Untyped clients
  // (Database = any) instantiate these arguments as `any` even though the
  // table-name literal is still echoed into the generic — so never match on
  // return-type text.
  try {
    const rt = fromCallExpr.getReturnType();
    for (const typeArg of rt.getTypeArguments()) {
      if (typeArg.isAny() || typeArg.isUnknown()) continue;
      const rowProp = typeArg.getProperty('Row');
      if (!rowProp) continue;
      const rowType = rowProp.getTypeAtLocation(fromCallExpr);
      if (rowType.isAny() || rowType.isUnknown()) continue;
      if (rowType.getProperties().length > 0) return true;
    }
  } catch { /* fall through */ }

  // Strategy 2 (bug-fixed): explicit type argument at the client binding,
  // e.g. `const sb = createClient<Database>(...)`. NOTE: use the IDENTIFIER
  // symbol (clientExpr.getSymbol()), not clientExpr.getType().getSymbol() —
  // the type's symbol is the SupabaseClient interface declaration and the
  // VariableDeclaration branch never ran.
  try {
    const propAccess = fromCallExpr.getExpression();
    if (propAccess.getKind() === SyntaxKind.PropertyAccessExpression) {
      const clientExpr = (propAccess as PropertyAccessExpression).getExpression();
      for (const decl of clientExpr.getSymbol()?.getDeclarations() ?? []) {
        if (decl.getKind() !== SyntaxKind.VariableDeclaration) continue;
        const init = (decl as VariableDeclaration).getInitializer();
        if (init?.getKind() === SyntaxKind.CallExpression &&
            (init as CallExpression).getTypeArguments().length > 0) return true;
      }
    }
  } catch { /* fall through */ }
  return false;
}

table becomes unused by the heuristic — either drop the parameter (touching column-reads.ts:180 / column-writes.ts:218) or keep it for an optional belt-and-braces assertion that some type arg is a string-literal type equal to table (not required: overload resolution guarantees the Relation belongs to the queried table). Also apply the same identifier-symbol fix to the rpc copy at column-reads.ts:266 — best extracted as a shared clientBindingHasExplicitTypeArgs(clientExpr) in supabase-shared.ts used by both.

Fixture changes (fixtures are the correctness oracle per PRODUCT inv 23/24, so the stub must reproduce the real-world shape):

1. Upgrade __tests__/fixtures/07-column-reads/supabase-stub.ts and 08-column-writes/supabase-stub.ts to mirror supabase-js 2.105.x: default DB = any; from<TN extends string>(table: TN) returns QueryBuilder<ClientOptions, Schema, Relation, TN> where Relation is Schema['Tables'][TN] ({ Row; Insert; Update }) for typed DB and any for untyped — the critical property being the echoed TN generic on the untyped path. Gate on the existing untyped fixtures: after the stub upgrade but before the heuristic fix, column-reads.test.ts:64-102 (“isTyped=false / indirect”) MUST FAIL via branch 1-a — proving the stub now reproduces the defect — and pass again after the fix.
2. New false-positive fixtures (each with a header comment stating expected rows, matching house style):
   • 07-column-reads/untyped-structural-client.ts — hand-rolled builder with inferred structural return type (makeDb() returning object literals), .from('bid_questions').select('project_id') + .eq('project_id', …). Expected: isTyped=false, confidence='indirect'. This is the branch-1b regression case (its return-type text is { select(...): ... }).
   • 07-column-reads/untyped-param-client.ts — function f(client: SupabaseClient) { … } (bare stub type, no generic). Expected isTyped=false/indirect.
   • 07-column-reads/typed-param-client.ts — function f(client: SupabaseClient<Database>) { … }. Expected isTyped=true/exact (exercises strategy 1 across a function boundary — strategy 2 cannot see a parameter; also removes the doc-comment’s false claim at supabase-shared.ts:24-26 that boundary-crossing always degrades to indirect).
   • 08-column-writes/untyped-structural-insert.ts — hand-rolled builder .insert({ project_id: x }). Expected isTyped=false, confidence='indirect'.
3. New tests: four it() blocks in column-reads.test.ts (structural→indirect, bare-param→indirect, typed-param→exact, untyped stub-client still indirect post-stub-upgrade) and one in column-writes.test.ts (structural insert→indirect). Existing typed-fixture assertions (isTyped=true/exact) must keep passing — they will, via upgraded-stub strategy 1 and fixed strategy 2.

Expected before/after on the real repo (verification commands for the implementer): bun run ast-dataflow column-reads --table source_documents --column id --json — scripts/eval-classification.ts:228 and e2e/global-teardown.ts:65,94 flip exact→indirect / isTyped false; lib/dashboard.ts:487 and lib/topic-inference.ts:148 stay exact. No rows disappear (inv 24 safe); inv 15 semantics restored.

B) Spatial-coverage truncation

New file tools/ast-dataflow/truncate.ts (keep resolve.ts symbol-focused; a pure module is trivially unit-testable):

import type { BaseResult } from './types';

export interface SpatialTruncateResult<R extends BaseResult> {
  rows: R[];                 // ≤ limit, sorted by (file, line, column)
  truncated: boolean;        // allRows.length > limit
  totalEstimated: number | undefined; // allRows.length when truncated, else undefined (envelope convention)
}

const byFileLineCol = <R extends BaseResult>(a: R, b: R): number =>
  a.file < b.file ? -1 : a.file > b.file ? 1 : a.line - b.line || a.column - b.column;

export function truncateSpatial<R extends BaseResult>(
  allRows: readonly R[],
  limit: number,
): SpatialTruncateResult<R> {
  const sorted = [...allRows].sort(byFileLineCol);
  if (sorted.length <= limit) return { rows: sorted, truncated: false, totalEstimated: undefined };
  // Group by file; Map insertion order = file-sorted order.
  const byFile = new Map<string, R[]>();
  for (const r of sorted) {
    const g = byFile.get(r.file); if (g) g.push(r); else byFile.set(r.file, [r]);
  }
  const groups = [...byFile.values()];
  // Round-robin: round 0 takes each file's first hit in file order (distinct
  // files first); round k takes each file's (k+1)-th hit (multi-hit files
  // thin last — a heavy file's LATEST hits are dropped first).
  const picked: R[] = [];
  outer: for (let round = 0; ; round++) {
    let pushed = false;
    for (const hits of groups) {
      if (round >= hits.length) continue;
      picked.push(hits[round]); pushed = true;
      if (picked.length === limit) break outer;
    }
    if (!pushed) break; // defensive; unreachable when total > limit
  }
  picked.sort(byFileLineCol);
  return { rows: picked, truncated: true, totalEstimated: allRows.length };
}

Guarantees: (1) deterministic output independent of ts-morph discovery order — total order on (file, line, column); ES stable sort covers exact ties by input order; (2) if distinct-file count ≤ limit, every file is represented; if it exceeds limit, the first limit files lexicographically get one row each; (3) truncated/totalEstimated computed from the full collected set, matching the totalEstimated-only-when-truncated envelope idiom (types.ts:57; undefined serialises away in JSON.stringify as today).

Per-query integration diff (mechanical, same shape everywhere). Collection: delete totalEstimated++ and the if (rows.length >= limit) continue / if (rows.length < limit) guard so every hit is pushed (in column-reads/column-writes this moves getLineAndColumnAtPos outside the guard — it now runs for all hits; fine at corpus scale, see perf note). Envelope:

const t = truncateSpatial(rows, limit);
return { query: '…', args: { ...args, limit }, results: t.rows,
         truncated: t.truncated, totalEstimated: t.totalEstimated,
         durationMs: Date.now() - started };

Apply to: callers.ts (84-101), references.ts (171-172), importers.ts (256, 313 — single truncate after both passes), column-reads.ts (231, 288), column-writes.ts (236), enum-uses.ts (122, 146, 214), dead-exports.ts (227), string-literal-uses.ts (157, 190), type-evolution.ts (also delete the three break-at-limit guards in findPropertySites at 193/199/216 — fixes the totalEstimated undercount), type-drift-detect.ts (752-754; then re-apply the classification sort at 789 AFTER truncation — stable sort preserves file/line order within each class, keeping the Markdown report sections intact).

Exempt (document in each file’s header and in TECH.md): flow-trace.ts — rows are ordered hops of a trace path, truncation must stay traversal-ordered (perf tests assert hop sequences, performance.test.ts:56/100); reexport-chain.ts — rows are chain hops (declaration → barrel(distance n) → importers) where spatial reorder breaks chain contiguity; keep its collect-then-slice, but document that its totalEstimated is a lower bound because the BFS at line 128 stops enumerating at the limit.

Tests:

• New __tests__/truncate.test.ts (pure unit, no ts-morph): (1) under-limit returns all rows file/line/column-sorted, truncated:false, totalEstimated undefined; (2) over-limit, distinct files ≤ limit → every file represented; (3) skew case — fileA×150 + 50 files×2, limit 100 → all 51 files present, fileA keeps its earliest lines; (4) limit < distinct files → first limit files lexicographically, one row each; (5) determinism — shuffled input yields identical output; (6) totalEstimated = full count exactly; (7) same-file/line tie broken by column.
• One integration test in string-literal-uses.test.ts (or column-reads.test.ts): low limit against a fixture set spanning ≥2 files with multiple hits each → assert both files appear in results and totalEstimated equals the un-truncated total (the current first-come code fails this).
• Ordering ripple: because un-truncated results are now sorted (previously discovery order), re-run the ast-dataflow suite; the survey shows tests are already order-agnostic (they .sort() first) except single-row results[0] assertions (callers.test.ts:101, type-drift-detect.test.ts:338) which are unaffected. Also re-run performance.test.ts: collecting all rows before truncation adds getLineAndColumnAtPos per hit past the old cap — well within the 10 s heuristic budget (inv 19) at the ~1.4k-file corpus; if a pathological needle (e.g. string-literal 'id') regresses, the documented fallback is a two-phase variant that sorts on (file, node position) and materialises line/column only for the picked rows — do not build it pre-emptively.

Sequencing note: land B’s helper + conversions first (pure mechanics, big test surface), then A (fixture stub upgrade → watch the untyped tests fail via branch 1-a → land the new detectIsTyped + strategy-2 fix → suite green), so the stub upgrade’s defect-reproduction step is observable in isolation.