Skip to content

Tighten isVisualPath so a non-visual apps/*/ file (e.g. a .ts lib file) doesn't trigger screenshot capture #6322

Description

@JSONbored

Problem

isVisualPath (src/review/visual/paths.ts) gates whether the visual before/after screenshot-capture pipeline runs at all for a PR. Its first pattern, /^apps\/[^/]+\//i, matches ANY file under ANY apps/*/ folder regardless of extension — so a pure .ts logic/lib file (e.g. apps/ui/src/lib/metagraphed/queries.ts, JSONbored/metagraphed#6036) triggers full capture even though it touches nothing renderable.

The module's own doc comment claims "A backend change (.ts/.md/.json/.py/...) matches NONE of these, so capture never triggers for it" — that's false for any backend file that happens to live inside a frontend app's folder tree.

This wastes Browser Rendering + R2 storage on every non-visual PR under apps/*/, and produces a confusing "Visual preview" table (with a permanently-broken "after" placeholder — see #6117-style companion issues) on PRs the bot's own review text correctly calls non-visual.

Area

src/review/visual/paths.ts (isVisualPath, VISUAL_PATTERNS).

Proposal

Tighten pattern 1 so an apps/*/ path only counts as visual when it ALSO matches a front-end signal — either a route/page-style path segment (mirroring capture.ts's own DEFAULT_ROUTE_FILE = /apps\/[^/]+\/src\/routes\/(.+?)\.(?:tsx|jsx)$/i) or one of pattern 3's front-end extensions (.tsx/.jsx/.css/.scss/.sass/.less/.html/.svg/.astro/.vue/.svelte/.mdx). A pure apps/*/**/*.ts file (no route/page shape, no front-end extension) should return false.

Cross-check the result against screenshot-table-gate.ts's own (config-driven, whenPaths) visual-relevance decision for a handful of representative repos/paths — the two classifiers currently disagree, and isVisualPath should never be BROADER than what the gate itself considers in-scope for screenshot evidence.

Deliverables

  • isVisualPath no longer matches a pure backend/lib .ts (or other non-front-end-extension) file just because it lives under apps/*/
  • Existing route/page/asset cases (.tsx, public/**, etc.) remain matched — this is a narrowing, not a rewrite
  • New tests: a lib/.ts file under apps/ui/src/lib/**false; a route file under apps/ui/src/routes/**true; the existing public/** and front-end-extension cases still true
  • Regression test reproducing the exact fix(ui): apply the pre-2000 placeholder rule in normalizeFreshnessSources metagraphed#6036 file path (apps/ui/src/lib/metagraphed/queries.ts) → false

Resources

Boundaries

Scope is isVisualPath only. Don't touch screenshot-table-gate.ts's own config-driven scoping, and don't change the capture pipeline itself (that's this milestone's other issues). Maintainer-only — assigned JSONbored.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions