Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/review/visual/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,21 @@
// EMPHATIC gate: screenshots fire ONLY for WEB-VISIBLE changes — any frontend app folder (apps/*/**, e.g.
// apps/loopover-ui/** or apps/ui/**), a public asset (public/**, e.g. an OG image), or a front-of-house
// source extension (.tsx/.jsx/.css/.scss/.sass/.less/.html/.svg/.astro/.vue/.svelte/.mdx). A backend change
// (.ts/.md/.json/.py/...) matches NONE of these, so capture never triggers for it.
// OUTSIDE an app folder (.ts/.md/.json/.py/... under e.g. src/**) matches NONE of these, so capture never
// triggers for it.
//
// PURE — no imports, no I/O. Callers MUST filter changed files through this before any capture.
// #6322: pattern 1's app-folder prefix alone is extension-agnostic, so a bare .ts file living INSIDE an app's
// own src/ tree (a data/logic/hooks/test file — TanStack Router's file-based routes are always .tsx/.jsx, so a
// bare .ts can never itself be a route) also matched, even though it renders nothing (confirmed live:
// JSONbored/metagraphed#6036 touched only apps/ui/src/lib/metagraphed/queries.ts + its .test.ts sibling, a
// non-visual data-layer fix the bot's own screenshot-table-gate correctly called out of scope, yet capture
// still ran and burned Browser Rendering on a meaningless "before/after of the unchanged homepage"). Excluding
// apps/*/src/**/*.ts specifically (not .tsx) fixes exactly that case while leaving every existing scope
// untouched: an app-ROOT .ts config file (tailwind.config.ts, vite.config.ts — genuinely can affect rendered
// output) still matches pattern 1 since it isn't under src/; README.md/components.json/any non-.ts file
// anywhere in the app tree still matches (not .ts); every .tsx/.jsx route or component is unaffected (this
// exclusion only strips bare .ts, never .tsx).
const NON_VISUAL_APP_SOURCE = /^apps\/[^/]+\/src\/.*\.ts$/i;

const VISUAL_PATTERNS: RegExp[] = [
/^apps\/[^/]+\//i,
Expand All @@ -17,7 +29,10 @@ const VISUAL_PATTERNS: RegExp[] = [
];

/** True when `path` is a web-visible change worth screenshotting (frontend page / public OG asset / front-end
* source file). Backend .ts/.md/.json/.py paths return false → capture must NOT trigger for them. */
* source file). Backend .ts/.md/.json/.py paths return false → capture must NOT trigger for them, and (#6322)
* so does a bare .ts file under an app's own src/ tree even though it starts with an app-folder prefix — see
* NON_VISUAL_APP_SOURCE's doc comment above for why that specific case is safe to exclude. */
export function isVisualPath(path: string): boolean {
if (NON_VISUAL_APP_SOURCE.test(path)) return false;
return VISUAL_PATTERNS.some((pattern) => pattern.test(path));
}
21 changes: 21 additions & 0 deletions test/unit/visual-paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,25 @@ describe("isVisualPath (web-visible-only capture gate)", () => {
// A .ts (backend) must still be false even upper-cased — it is not a web-visible extension.
expect(isVisualPath("src/Worker.TS")).toBe(false);
});

it("does NOT match a bare .ts file under an app's own src/ tree (#6322 — logic/hooks/tests, never a route)", () => {
// The exact JSONbored/metagraphed#6036 case: a non-visual data-layer fix that still burned capture
// budget before this fix, on files the bot's own screenshot-table-gate correctly called out of scope.
expect(isVisualPath("apps/ui/src/lib/metagraphed/queries.ts")).toBe(false);
expect(isVisualPath("apps/ui/src/lib/metagraphed/queries.test.ts")).toBe(false);
expect(isVisualPath("apps/loopover-ui/src/hooks/useSession.ts")).toBe(false);
expect(isVisualPath("apps/loopover-ui/src/types.ts")).toBe(false);
// Case-insensitivity applies to the exclusion too, not just the positive matches above.
expect(isVisualPath("APPS/UI/SRC/LIB/QUERIES.TS")).toBe(false);
});

it("still matches everything the exclusion must NOT touch", () => {
// A .tsx file under the same src/ tree is a route/component -- the exclusion only strips bare .ts.
expect(isVisualPath("apps/ui/src/lib/metagraphed/Queries.tsx")).toBe(true);
// An app-ROOT .ts config file (not under src/) can genuinely affect rendered output -- still in scope.
expect(isVisualPath("apps/ui/tailwind.config.ts")).toBe(true);
expect(isVisualPath("apps/loopover-ui/vite.config.ts")).toBe(true);
// A non-.ts file under src/ (docs, json, etc.) is untouched by this exclusion.
expect(isVisualPath("apps/ui/src/lib/metagraphed/README.md")).toBe(true);
});
});