Skip to content

Commit 69e868c

Browse files
committed
Delete the tolerant ?? records aliases and narrow the guard that blessed them
`ObjectStackAdapter.find()` resolves a normalized `QueryResult` whose sole non-empty return path (`normalizeQueryResult`) builds an object literal with exactly `data`, `total`, `page`, `pageSize`, `hasMore`. `records` is a key it READS off the transport envelope, never one it writes -- so the `?? .records` limb in both surviving repairs is unreachable by contract. Behaviour-preserving: `.data` is read first and always wins today. What goes is a spelling the producer cannot emit, sitting in the page a customer copies from. The third piece is what stops it returning: `recordsOnlyReads()` skipped any line carrying `.data`, which made `data ?? records` the one shape it could not see. That carve-out is replaced by comment/string stripping, and the detector renamed `recordsReads` since "only" no longer describes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
1 parent 2cce3fd commit 69e868c

3 files changed

Lines changed: 109 additions & 17 deletions

File tree

content/docs/ui/react-pages.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ function Page() {
142142
$filter: ['status', '!=', 'paid'],
143143
$top: 200,
144144
});
145-
const records = result?.data ?? result?.records ?? (Array.isArray(result) ? result : []);
145+
const records = result?.data ?? (Array.isArray(result) ? result : []);
146146
if (alive) setRows(records);
147147
})();
148148
return () => { alive = false; };

examples/app-showcase/src/ui/pages/crm-workbench.page.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ function Page() {
3535
// a normalized QueryResult with a "data" array, NOT the REST envelope with
3636
// a "records" array. Reading .records here always missed, so the KPI cards
3737
// silently stuck at 0 even though the ListView beside them showed the same
38-
// rows. Read .data first, with .records/array fallbacks for robustness.
38+
// rows. Read .data: it is the only row shape QueryResult declares, so a
39+
// tolerant '.data || .records' alias would render correctly while
40+
// teaching a spelling the producer cannot emit.
3941
//
4042
// The cap is $top, not 'limit': QueryParams declares only $-prefixed keys
4143
// and the adapter copies only those, so a bare 'limit' is dropped without
@@ -46,7 +48,7 @@ function Page() {
4648
// "Active" stays a per-row verdict over the 200 rows actually fetched;
4749
// an exact one would need its own filtered count query.
4850
const all = await adapter.find('showcase_project', { $top: 200 });
49-
const rows = Array.isArray(all) ? all : (all && (all.data || all.records)) || [];
51+
const rows = Array.isArray(all) ? all : (all && all.data) || [];
5052
const total = typeof (all && all.total) === 'number' ? all.total : rows.length;
5153
setStats({ total, active: rows.filter((r) => r.status === 'active').length });
5254
} catch (e) { console.warn('[CRM Workbench] failed to refresh stats', e); }

scripts/check-react-page-adapter-contract.mjs

Lines changed: 104 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
//
5555
// The detectors below are MOVED, not copied. The app-showcase sweep left that
5656
// test file in the same edit that added this gate, because two copies of
57-
// `recordsOnlyReads()` would double the places a future fix has to land --
57+
// `recordsReads()` would double the places a future fix has to land --
5858
// which is the shape of the defect above, not a fix for it. The test keeps the
5959
// half a text scan cannot do: it EXECUTES the renewals-pipeline rollup against
6060
// a contract-faithful adapter double.
@@ -137,8 +137,13 @@ const CENSUS_ANCHORS = {
137137
};
138138

139139
// ---------------------------------------------------------------------------
140-
// The two detectors -- MOVED verbatim from
140+
// The two detectors -- MOVED from
141141
// examples/app-showcase/test/react-page-adapter-query-contract.test.ts (#10288).
142+
// `unprefixedQueryKeys` is still verbatim. `recordsReads` is NOT: it arrived
143+
// carrying a `.data`-beside carve-out that made `data ?? records` invisible,
144+
// and that carve-out was narrowed to comment/string stripping in the same
145+
// edit that deleted the two aliases it was load-bearing for. See the
146+
// function's own header for why a tolerant alias is a finding.
142147
// ---------------------------------------------------------------------------
143148

144149
const DECLARED_QUERY_PARAM_PREFIX = '$';
@@ -223,28 +228,77 @@ export function unprefixedQueryKeys(source) {
223228
return found;
224229
}
225230

231+
/** A `.records` / `?.records` PROPERTY read -- not the bare word, not a longer name. */
232+
const RECORDS_READ = /\??\.\s*records\b/;
233+
226234
/**
227-
* A `.records` read with no `.data` beside it, off a find() result.
235+
* One line's executable text: string and template bodies blanked (quotes kept),
236+
* and a trailing `//` or block comment dropped.
237+
*
238+
* This is what lets the `.data`-beside carve-out go without the detector
239+
* starting to fire on text that merely SPELLS the trap: a webhook payload
240+
* naming `'data.records.updated'`, or a line whose trailing comment explains
241+
* why `.records` is wrong. Both are kept out of the sweep by the SELECTOR
242+
* today, and a detector that is quiet only because of the selector is one
243+
* population change away from firing.
244+
*
245+
* @param {string} line
246+
* @returns {string}
247+
*/
248+
export function codeOnly(line) {
249+
let out = '';
250+
let quote = null;
251+
for (let i = 0; i < line.length; i++) {
252+
const c = line[i];
253+
if (quote !== null) {
254+
if (c === '\\') { i++; continue; }
255+
if (c === quote) { quote = null; out += c; }
256+
continue;
257+
}
258+
if (c === '"' || c === "'" || c === '`') { quote = c; out += c; continue; }
259+
if (c === '/' && (line[i + 1] === '/' || line[i + 1] === '*')) break;
260+
out += c;
261+
}
262+
return out;
263+
}
264+
265+
/**
266+
* Every `.records` read off a find() result, judged on the line's CODE.
228267
*
229268
* `find()` resolves to a normalized `QueryResult` -- rows under `data`, never
230269
* the REST envelope's `records`. Reading `.records` yields `undefined` on every
231270
* call, so a KPI over it sticks at 0 forever while the `<ListView>` beside it
232271
* shows the same rows correctly.
233272
*
234-
* Comment lines are skipped: a page that explains the trap in prose (and
235-
* `crm-workbench` does, right above the call it once got wrong) is documenting
236-
* the contract, not violating it. The read itself is what this looks for.
273+
* Whole-line comments are skipped and `codeOnly()` strips the rest: a page that
274+
* explains the trap in prose (and `crm-workbench` does, right above the call it
275+
* once got wrong) is documenting the contract, not violating it. The read
276+
* itself is what this looks for.
277+
*
278+
* ⛔ A `.data` READ BESIDE IT IS NOT AN EXEMPTION. This detector used to skip
279+
* any line carrying `.data`, which made `result.data ?? result.records` the one
280+
* shape it could not see -- and that is the shape BOTH surviving repairs of
281+
* this defect had landed as, including the sample in
282+
* `content/docs/ui/react-pages.mdx` that a customer copies from. A tolerant
283+
* alias renders correctly today (`.data` is read first and always wins), so
284+
* nothing is on fire; what it does is teach authors and code assistants a
285+
* spelling the producer cannot emit, leaving the next author who simplifies
286+
* the chain to guess which limb was real. That guess is how ONE wrong read
287+
* reached three files.
288+
*
289+
* Measured before the narrowing landed: the carve-out was load-bearing for
290+
* exactly two lines across both populations -- the two aliases deleted in this
291+
* same edit -- and nothing else. So it reds no bystander.
237292
*
238293
* @param {string} source
239294
* @returns {string[]}
240295
*/
241-
export function recordsOnlyReads(source) {
296+
export function recordsReads(source) {
242297
const out = [];
243298
for (const line of source.split('\n')) {
244299
const trimmed = line.trim();
245300
if (trimmed.startsWith('//') || trimmed.startsWith('*') || trimmed.startsWith('/*')) continue;
246-
if (!trimmed.includes('.records')) continue;
247-
if (trimmed.includes('.data')) continue;
301+
if (!RECORDS_READ.test(codeOnly(trimmed))) continue;
248302
out.push(trimmed);
249303
}
250304
return out;
@@ -431,7 +485,7 @@ export function sweep(population) {
431485
+ `Spell it \`${key === 'limit' ? 'top' : key}\`. In: ${snippet}`,
432486
);
433487
}
434-
for (const line of recordsOnlyReads(source)) {
488+
for (const line of recordsReads(source)) {
435489
findings.push(
436490
`${at(lineOfText(source, line))}: reads \`.records\` off a find() result. \`QueryResult\` `
437491
+ `declares rows under \`data\` — \`.records\` is \`undefined\` on every call, forever, `
@@ -511,8 +565,8 @@ export function selfTest() {
511565
'unprefixedQueryKeys fires on a known-bad source, naming both dropped keys',
512566
);
513567
assert(
514-
JSON.stringify(recordsOnlyReads(bad)) === JSON.stringify(['const rows = (a && a.records) || [];']),
515-
'recordsOnlyReads fires on a known-bad source, and a COMMENT mentioning .records is not a read',
568+
JSON.stringify(recordsReads(bad)) === JSON.stringify(['const rows = (a && a.records) || [];']),
569+
'recordsReads fires on a known-bad source, and a COMMENT mentioning .records is not a read',
516570
);
517571

518572
// ...and stay silent on the corrected shape, so a green means something.
@@ -521,7 +575,43 @@ export function selfTest() {
521575
const rows = a.data ?? [];
522576
`;
523577
assert(unprefixedQueryKeys(good).length === 0, 'unprefixedQueryKeys is silent on the corrected shape');
524-
assert(recordsOnlyReads(good).length === 0, 'recordsOnlyReads is silent on the corrected shape');
578+
assert(recordsReads(good).length === 0, 'recordsReads is silent on the corrected shape');
579+
580+
// ── The narrowing: a `.data` read BESIDE it is not an exemption ───────────
581+
// The first two are the two tolerant aliases the `.data`-beside carve-out
582+
// used to bless -- the app-showcase one and, worse, the docs sample a
583+
// customer copies from. Both rendered correctly while teaching a spelling
584+
// `ObjectStackAdapter.find()` cannot emit, which is what the carve-out cost.
585+
assert(
586+
recordsReads(`const rows = Array.isArray(all) ? all : (all && (all.data || all.records)) || [];`).length === 1,
587+
'a `data || records` alias IS a finding — the carve-out that blessed it is what let BOTH surviving repairs land as tolerance',
588+
);
589+
assert(
590+
recordsReads(`const records = result?.data ?? result?.records ?? (Array.isArray(result) ? result : []);`).length === 1,
591+
'the docs sample\'s `?? result?.records` alias IS a finding — optional chaining is a read',
592+
);
593+
assert(
594+
recordsReads(`const records = result?.data ?? (Array.isArray(result) ? result : []);`).length === 0,
595+
'the REPAIRED docs sample is silent — a local named `records` is not a `.records` read',
596+
);
597+
598+
// ...and the narrowing must not start firing on text that merely SPELLS it.
599+
assert(
600+
recordsReads(`emit({ type: 'data.records.updated' });`).length === 0,
601+
'a webhook payload naming data.records.updated in a STRING is not a read — the detector no longer leans on the selector for this',
602+
);
603+
assert(
604+
recordsReads(`const rows = result.data ?? []; // never .records — QueryResult does not declare it`).length === 0,
605+
'a TRAILING comment naming .records beside a canonical read is not a finding — prose documenting the trap is not the trap',
606+
);
607+
assert(
608+
recordsReads(`const n = result.recordsCount;`).length === 0,
609+
'a LONGER property is not a `.records` read — the detector matches a whole property name',
610+
);
611+
assert(
612+
codeOnly(`a.records // '.data'`) === 'a.records ' && codeOnly(`x('.records')`) === `x('')`,
613+
'codeOnly drops a trailing comment and blanks string BODIES while keeping the quotes',
614+
);
525615

526616
// ── The contracts this sweep must NOT fabricate findings on ─────────────
527617
// Both are real lines from `content/docs`, and both are CORRECT where they sit.
@@ -530,7 +620,7 @@ export function selfTest() {
530620
'an ObjectQL `engine.find` is a different contract — its unprefixed keys are not findings',
531621
);
532622
assert(
533-
recordsOnlyReads(` return data?.records.map(a => <div key={a.id}>{a.name}</div>);`).length === 1,
623+
recordsReads(` return data?.records.map(a => <div key={a.id}>{a.name}</div>);`).length === 1,
534624
'the detector itself DOES flag a bare .records read — so the client-sdk exclusion has to happen in the SELECTOR',
535625
);
536626
assert(

0 commit comments

Comments
 (0)