Skip to content

Commit 1ea199a

Browse files
atulmguptaCopilot
andcommitted
fix(web): repoint and harden rotted frontend audit targets
Five of the ten failing audits were not code defects, they were audit rot. PR #64 (refactor/filters) renamed or deleted several pages these audits point at, and the audits silently skipped the very files they exist to guard. Path rot: - audit-inline-help: the notification-channels target pointed at a deleted NotificationChannelsView.tsx. Repointed at components/channels/ so further decomposition cannot rot it again. Coverage went 22 -> 27 (min 25) from the repoint alone, meaning the audit had been passing vacuously. - audit-virtualization, audit-export-adoption: repointed post-rename paths. Logic bugs: - audit-deferred-filter grepped the *page* file for React.memo on a row component defined elsewhere, so an already-memoised DriveCard read as a violation. Targets now carry rowComponentPath and the check reads the component's defining file, accepting `export const X = memo(XImpl, ...)`. - audit-filterbar-chips did not skip co-located *.test.tsx, contradicting its own documented __tests__ skip intent. Anti-rot: stale PENDING_MIGRATION paths in audit-virtualization and audit-export-adoption now hard-fail instead of printing "(file missing)" and exiting 0. A path that no longer exists is now a build break, not a silent pass, so this class of decay cannot recur unnoticed. Also adds two justified allow-list entries: RadioCard.tsx to audit-sr-only (identical `peer sr-only` case to the already-exempt Checkbox.tsx, where VisuallyHidden would break the peer sibling relationship) and lib/cn.test.ts to auditMotionTokens (the test asserts on raw duration-* strings by design). Every change here was mutation-verified: break the rule -> exit 1, restore -> exit 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 749f362 commit 1ea199a

7 files changed

Lines changed: 86 additions & 11 deletions

web/scripts/audit-deferred-filter.mjs

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,15 @@ const ROOT = 'src';
3636

3737
// Each TARGET is a path (relative to web/) and an optional row component
3838
// name. When `rowComponent` is set, the audit also requires that name
39-
// to appear inside a `memo(` call somewhere in the file — otherwise
40-
// the row will still re-render on every filter keystroke even though
41-
// the filter compute is deferred.
39+
// to be wrapped in `memo(` — otherwise the row will still re-render on
40+
// every filter keystroke even though the filter compute is deferred.
41+
//
42+
// `rowComponentPath` says WHERE that memo() wrapper lives. Row components
43+
// are routinely extracted out of the page into their own file (one exported
44+
// component per file, per the repo's monolith rule), and the memo() wrapper
45+
// travels with them. Without this the audit greps the page file, finds no
46+
// `memo(`, and reports a false positive against a component that is in fact
47+
// correctly memoised. Defaults to the page itself for inline row components.
4248
//
4349
// Server-driven targets are still listed: they must carry the
4450
// `deferred-filter:no` justification or the audit fails. That keeps
@@ -47,6 +53,7 @@ const TARGETS = [
4753
{
4854
path: join(ROOT, 'features', 'driving', 'pages', 'DrivesListPage.tsx'),
4955
rowComponent: 'DriveCard',
56+
rowComponentPath: join(ROOT, 'features', 'driving', 'components', 'DriveCard.tsx'),
5057
},
5158
{
5259
path: join(ROOT, 'features', 'system', 'pages', 'CommandHistoryPage.tsx'),
@@ -146,14 +153,39 @@ function auditFile(target) {
146153

147154
// Row-component memo() check — only when the target declares one.
148155
if (rowComponent) {
149-
const memoNames = memoCallNamesIn(text);
150-
if (!memoNames.has(rowComponent)) {
156+
// Resolve where the component is actually defined. An extracted row
157+
// component keeps its memo() wrapper in its own file, not in the page.
158+
const declPath = target.rowComponentPath ?? path;
159+
if (!existsSync(declPath)) {
160+
offenders.push({
161+
where: declPath,
162+
why:
163+
`Declared \`rowComponentPath\` for \`${rowComponent}\` does not ` +
164+
`exist. Update web/scripts/audit-deferred-filter.mjs to re-point ` +
165+
`this entry at the file that defines and memoises the row.`,
166+
});
167+
return;
168+
}
169+
170+
const declText = declPath === path ? text : readFileSync(declPath, 'utf8');
171+
172+
// Accept either form:
173+
// export const DriveCard = memo(DriveCardImpl, areEqual) ← extracted
174+
// const DriveCard = memo(function DriveCard() {...}) ← inline
175+
// The first wraps a differently-named impl, so a plain
176+
// `memo(DriveCard` substring search misses it.
177+
const exportedMemo = new RegExp(
178+
`(?:export\\s+)?const\\s+${rowComponent}\\s*(?::[^=]+)?=\\s*memo\\s*\\(`,
179+
).test(declText);
180+
const memoNames = memoCallNamesIn(declText);
181+
182+
if (!exportedMemo && !memoNames.has(rowComponent)) {
151183
const declLine = locOfMatch(
152-
text,
184+
declText,
153185
new RegExp(`function\\s+${rowComponent}\\b|const\\s+${rowComponent}\\b`),
154186
);
155187
offenders.push({
156-
where: declLine ? `${path}:${declLine}` : path,
188+
where: declLine ? `${declPath}:${declLine}` : declPath,
157189
why:
158190
`Row component \`${rowComponent}\` must be wrapped in ` +
159191
`\`memo(${rowComponent}, areEqual?)\` so unchanged rows skip ` +

web/scripts/audit-export-adoption.mjs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ const TARGET_FRAGMENTS = [
6868
// backlog stays visible. NOT a failure — there's nothing to enforce
6969
// `exportable` on if there's no <DataTable/>.
7070
const PENDING_MIGRATION = [
71-
'features/notifications/pages/NotificationsPage.tsx',
71+
// Successor to the deleted NotificationsPage.tsx (renamed in #64
72+
// "Refactor/filters"). InboxPage is a thin shell; rows are mapped into
73+
// <NotificationRow> inside InboxBody, so that is what must migrate.
74+
'features/notifications/components/InboxBody.tsx',
7275
'features/notifications/pages/AlertRulesPage.tsx',
7376
'features/admin/pages/ApiLogsPage.tsx',
7477
'features/driving/pages/DrivesListPage.tsx',
@@ -265,11 +268,19 @@ if (skippedNoDataTable.length > 0) {
265268
// PENDING_MIGRATION — informational only. Mirror the pattern used by
266269
// audit-virtualization.mjs: surface backlog so future sweeps can
267270
// migrate raw `.map()` / `<table>` rows to <DataTable/> + exportable.
271+
//
272+
// A *stale* entry is a hard failure, not a warning: a path that no longer
273+
// exists silently stops auditing the surface it was meant to track, which
274+
// is how NotificationsPage.tsx rotted through #64.
268275
const pendingMissing = [];
269276
for (const rel of PENDING_MIGRATION) {
270277
const full = path.join(ROOT, rel);
271278
if (!existsSync(full)) {
272279
pendingMissing.push(rel);
280+
failures.push({
281+
file: rel,
282+
reason: 'file not found (PENDING_MIGRATION is stale — repoint it at the renamed file or drop the entry)',
283+
});
273284
}
274285
}
275286
if (PENDING_MIGRATION.length > 0) {

web/scripts/audit-filterbar-chips.mjs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ function walk(dir) {
4646
continue;
4747
}
4848
if (!p.endsWith('.tsx')) continue;
49+
// Same rationale as the __tests__ skip above: co-located `*.test.tsx`
50+
// files mount <FilterBar> in isolation to exercise the primitive itself,
51+
// so requiring a sibling <ActiveFilterChips> there tests nothing real.
52+
if (p.endsWith('.test.tsx')) continue;
4953
auditFile(p);
5054
}
5155
}

web/scripts/audit-inline-help.mjs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,11 @@ const TARGETS = [
6262
},
6363
{
6464
name: 'notification-channels',
65-
path: 'src/features/notifications/components/NotificationChannelsView.tsx',
65+
// The former single-file NotificationChannelsView.tsx was decomposed into
66+
// this directory (ChannelCard / ChannelFormModal / ChannelProvidersPanel /
67+
// ChannelsGrid / ChannelStatsBand). Point at the directory so the target
68+
// survives further decomposition inside it.
69+
path: 'src/features/notifications/components/channels',
6670
min: 4,
6771
},
6872
];

web/scripts/audit-sr-only.mjs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,15 @@ const ROOT = join('src');
4545
* so Tailwind `peer-*:` variants on the indicator can read its state
4646
* while it stays in the accessibility tree. Wrapping it in
4747
* `<VisuallyHidden>` would break the `peer` sibling relationship.
48+
* - `RadioCard.tsx` is the same pattern for `<input type="radio">`: the
49+
* card body reads `peer-checked:` / `peer-focus-visible:` off the native
50+
* input, so the input has to stay a direct `peer` sibling.
4851
*/
4952
const ALLOWED_FILES = new Set([
5053
toAllowKey(join('src', 'components', 'a11y', 'VisuallyHidden.tsx')),
5154
toAllowKey(join('src', 'components', 'a11y', '__tests__', 'VisuallyHidden.test.tsx')),
5255
toAllowKey(join('src', 'components', 'ui', 'Checkbox.tsx')),
56+
toAllowKey(join('src', 'components', 'ui', 'RadioCard.tsx')),
5357
]);
5458

5559
function toAllowKey(p) {

web/scripts/audit-virtualization.mjs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ const ROOT = path.resolve(process.cwd(), 'src');
4141
// given prompt (READ-only — modifications still go through the normal
4242
// allowlist gate).
4343
const HOT_TABLE_PAGES = [
44-
'features/notifications/pages/AlertsPage.tsx',
4544
'features/admin/pages/LiveLogsPage.tsx',
4645
'features/charging/pages/TeslaChargingSessionsPage.tsx',
4746
'features/charging/pages/TeslaChargingHistoryPage.tsx',
@@ -54,7 +53,14 @@ const HOT_TABLE_PAGES = [
5453
// component that doesn't exist; document the gap so the future work can
5554
// migrate them to DataTable+virtualized.
5655
const PENDING_MIGRATION = [
57-
'features/notifications/pages/NotificationsPage.tsx',
56+
// Successor to the deleted NotificationsPage.tsx (renamed in #64
57+
// "Refactor/filters"). The page itself is a thin shell — rows are mapped
58+
// into <NotificationRow> inside InboxBody, so that is what must migrate.
59+
'features/notifications/components/InboxBody.tsx',
60+
// Successor to the deleted AlertsPage.tsx (same PR). Renders rows via a
61+
// raw .map() rather than <DataTable/>, so it cannot satisfy HOT_TABLE_PAGES.
62+
// Currently bounded by client-side Pagination, hence migration not urgent.
63+
'features/notifications/pages/AlertsListPage.tsx',
5864
'features/admin/pages/ApiLogsPage.tsx',
5965
'features/driving/pages/DrivesListPage.tsx',
6066
'features/charging/pages/ChargingListPage.tsx',
@@ -221,11 +227,19 @@ if (exemptedWaiver.length > 0) {
221227
// PENDING_MIGRATION — informational only. These pages render long
222228
// lists via raw `.map()` and ought to migrate to DataTable+virtualized
223229
// in a follow-up. Surfaced as warnings so the backlog stays visible.
230+
//
231+
// A *stale* entry is a hard failure, not a warning: a path that no longer
232+
// exists silently stops auditing the surface it was meant to track, which
233+
// is how NotificationsPage.tsx/AlertsPage.tsx rotted through #64.
224234
const pendingMissing = [];
225235
for (const rel of PENDING_MIGRATION) {
226236
const full = path.join(ROOT, rel);
227237
if (!existsSync(full)) {
228238
pendingMissing.push(rel);
239+
failures.push({
240+
file: rel,
241+
reason: 'file not found (PENDING_MIGRATION is stale — repoint it at the renamed file or drop the entry)',
242+
});
229243
}
230244
}
231245
if (PENDING_MIGRATION.length > 0) {

web/scripts/auditMotionTokens.mjs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,14 @@ const PATTERN = /\bduration-(\d+)\b/g;
4848
// Files to skip (paths relative to web/src). The tokens module legitimately
4949
// references the raw `duration-NNN` form for documentation; excluding it
5050
// keeps the audit honest without forcing a circular reference.
51+
//
52+
// `lib/cn.test.ts` asserts that the custom `duration-fast|normal|slow` keys
53+
// were registered into twMerge's SAME class group as the built-in numeric
54+
// scale — proving that requires spelling a raw `duration-200` as test input.
55+
// It ships no CSS, so it cannot cause the timing drift this audit prevents.
5156
const IGNORED = new Set([
5257
'lib/tokens.ts',
58+
'lib/cn.test.ts',
5359
]);
5460

5561
const files = globSync(FILE_GLOB, { cwd: ROOT });

0 commit comments

Comments
 (0)