Skip to content

Commit 36389b1

Browse files
bloveclaude
andcommitted
fix(docs): close the loop-blind loader check and two overclaims
Verification of the previous commit found one mutation still green and one comment stating a false rationale. The loader mirror check read the candidate array's DECLARATION, so it could see a reordered literal but not what the loop did with it. Both `for (const c of [...candidates].reverse())` and an `unshift` after the literal flip which page renders while the declaration still reads correctly, and both passed. A check on where a value is declared says nothing about how it is used, and resolution is a property of the use. It now also pins that the loop consumes the array directly and that nothing else touches it. The enumeration detector's comment claimed the backtick-and-quote rule is what keeps the badge paragraph's next sentence out of the enumeration it comments on. It is not — sentence segmentation does, and writing that sentence in the exact shape the rule was said to exclude leaves the check green. The comment now says what the rule is actually for and records what it does not do. Also corrects the page's own account of the dot. The dot is `currentColor`, so it is the same hue as the label and renders the same grey: it is a peripheral channel, not a greyscale one, and the label remains what separates one tone from another. The sentence this branch replaced was therefore accurate about greyscale and wrong only by omission — a smaller defect than the previous commit message claimed. Adds the assertion that rule had been missing: nothing checked the toned-badge dot existed, so deleting it kept every suite green and silently re-falsified the page. Both halves mutation-proven. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent bae8dba commit 36389b1

4 files changed

Lines changed: 78 additions & 12 deletions

File tree

‎apps/website/app/docs/__tests__/nav.test.ts‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,16 @@ describe("documentation navigation", () => {
149149
// with `path.resolve` slipped through the same pair of holes: the fragments
150150
// are all still present, and the count only ever counted `path.join`.
151151
//
152-
// The array's own text answers all three questions at once — which
153-
// candidates, how many, in what order — and answers them about the thing
154-
// `resolveHref` actually reproduces rather than about tokens that happen to
155-
// appear near it.
152+
// The array's own text answers three of the four questions at once — which
153+
// candidates, how many, in what order as WRITTEN.
154+
//
155+
// It does not answer the fourth, which is what the loop does with them, and
156+
// that gap is checked separately below. Reading the declaration alone was
157+
// itself defeatable: `for (const c of [...candidates].reverse())` flips
158+
// precedence with the literal untouched, and an `unshift` after the literal
159+
// adds a highest-precedence candidate the declaration never mentions. Both
160+
// passed. A check on where a value is DECLARED says nothing about how it is
161+
// USED, and resolution is a property of the use.
156162
const candidates = /const candidates = \[([\s\S]*?)\];/.exec(LOADER_SOURCE);
157163

158164
expect(
@@ -171,6 +177,27 @@ describe("documentation navigation", () => {
171177
).toBe(
172178
'path.join(root, `${base}.mdx`), path.join(root, base, "index.mdx")',
173179
);
180+
181+
// And that the loop consumes the array UNMODIFIED, in declaration order.
182+
expect(
183+
LOADER_SOURCE.includes("for (const c of candidates) {"),
184+
"lib/docs/load.ts no longer iterates `candidates` directly. Anything " +
185+
"between the array and the loop — `.reverse()`, a spread, a sort — " +
186+
"changes which candidate wins while leaving the declaration above " +
187+
"looking correct. Mirror the new behaviour in `resolveHref`, then " +
188+
"update this check.",
189+
).toBe(true);
190+
191+
// Two mentions and no more: the declaration and the loop. A third is a
192+
// mutation of the list (`unshift`, `push`, `splice`) that the declaration
193+
// cannot show.
194+
expect(
195+
[...LOADER_SOURCE.matchAll(/\bcandidates\b/g)].length,
196+
"lib/docs/load.ts mentions `candidates` somewhere beyond declaring and " +
197+
"iterating it. If the list is mutated after it is built, the array " +
198+
"literal is no longer the resolution order — update `resolveHref` and " +
199+
"this check together.",
200+
).toBe(2);
174201
});
175202

176203
it("places Grouping directly after Filtering", () => {

‎apps/website/content/docs/grid/cell-presentations.mdx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,15 @@ spotting. The dot restores that as shape and colour at no cost to the label,
174174
since a dot is a graphical object and owes 3:1 rather than 4.5:1. An untoned
175175
chip draws no dot.
176176

177-
So both this and `PretableStatus` show a dot, and the chip is what separates
178-
them: a badge is a noun the row _is_ — a category, a flag — and reads as a
179-
bounded chip; a status is a state the row is _in_, and reads as a dot beside
180-
plain text.
177+
The dot is drawn in `currentColor`, so it is the same hue as the label. That
178+
makes it a _peripheral_ channel, not a greyscale one: it tells you at a glance
179+
which rows carry a tone, but in greyscale it renders the same grey as the label,
180+
and the label is still what distinguishes one tone from another.
181+
182+
So a toned chip and a `PretableStatus` both show a dot, and the chip is what
183+
separates them: a badge is a noun the row _is_ — a category, a flag — and reads
184+
as a bounded chip; a status is a state the row is _in_, and reads as a dot
185+
beside plain text.
181186

182187
`PretableBadgeTone` is `"positive"`, `"negative"`, `"warning"` or `"info"`.
183188
There is deliberately **no `neutral` member**: a badge with no tone is the

‎apps/website/lib/docs/__tests__/docs-api-surface.test.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ function stringUnionMembers(body: string): string[] | undefined {
203203
.split("|")
204204
.map((part) => part.trim());
205205

206-
if (parts.length === 0) return undefined;
206+
// No empty-array guard: `String.prototype.split` never returns one, so a
207+
// check for it would be a branch no input can reach.
207208
if (!parts.every((part) => /^"[^"]*"$/.test(part))) return undefined;
208209

209210
return parts.map((part) => part.slice(1, -1));
@@ -862,9 +863,16 @@ interface ProseEnumeration {
862863
* is not wrong, and this file's own history says a check that fails on a
863864
* non-defect is a check the next author deletes.
864865
*
865-
* The literal is required in backticks too, and quoted, which is what keeps the
866-
* badge paragraph's next sentence — "There is deliberately no `neutral`
867-
* member" — out of the enumeration it is commenting on.
866+
* The literal is required in backticks too, and quoted, so that a name-drop
867+
* with no members listed is not read as an enumeration of zero.
868+
*
869+
* It is NOT what keeps the badge paragraph's next sentence — "There is
870+
* deliberately no `neutral` member" — out of the enumeration it comments on;
871+
* an earlier draft of this comment claimed that, and it is wrong. Sentence
872+
* segmentation does it: the enumerating sentence ends at its own period, and
873+
* the next one is a separate candidate that never names the type in backticks.
874+
* Writing that next sentence as ``no `"neutral"` member`` — the exact shape the
875+
* quoting rule was supposed to exclude — leaves this check green.
868876
*
869877
* A sentence ends at the first `.` followed by whitespace or end of text, so a
870878
* decimal inside one (`-0.2`) does not cut it short.

‎packages/ui/src/__tests__/css-cascade.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,32 @@ describe("grid.css cascade contract", () => {
635635
`badge rule "${selector.trim()}" mixes a fill; the badge's fill is the grid surface, untinted`,
636636
).not.toMatch(/background[^;]*color-mix/);
637637
}
638+
639+
// The dot a TONED badge carries, which nothing asserted until now — and the
640+
// docs page describes it, so deleting this rule silently made that page
641+
// wrong. It exists because dropping the tinted fill above fixed the
642+
// contrast but left tone readable only by reading the label; the dot puts
643+
// it back as shape, so scanning a column is spotting again.
644+
const dot = css.match(
645+
/:where\(\[data-pretable-badge\]\[data-pretable-tone\]\)::before\s*\{([\s\S]*?)\}/,
646+
)?.[1];
647+
expect(
648+
dot,
649+
"no toned-badge ::before rule. The dot is the only tone channel that " +
650+
"survives peripheral vision, and `grid/cell-presentations.mdx` tells " +
651+
"readers it is there.",
652+
).toBeDefined();
653+
expect(dot).toMatch(/content:\s*""/);
654+
// `currentColor`, not a per-tone background: that is what makes the dot
655+
// track the label's tone with no rule per tone, and it is the claim the
656+
// docs page makes about greyscale (same hue as the label, so the label is
657+
// still what separates one tone from another).
658+
expect(dot).toMatch(/background:\s*currentColor/);
659+
660+
// Not also asserting the pseudo-element stays outside :where() — the
661+
// "no pseudo-element inside :where()" test below already covers every rule
662+
// in the file, and it fires on this one. A second copy here would be a
663+
// check with nothing of its own to catch.
638664
});
639665

640666
test("the entity's secondary line is dimmed by a token, never by opacity", () => {

0 commit comments

Comments
 (0)