Skip to content

#154 single-source constants sweep (7 items; item 4 dropped)#171

Merged
realproject7 merged 1 commit into
mainfrom
task/154-single-source-constants
Jul 14, 2026
Merged

#154 single-source constants sweep (7 items; item 4 dropped)#171
realproject7 merged 1 commit into
mainfrom
task/154-single-source-constants

Conversation

@realproject7

Copy link
Copy Markdown
Owner

Closes #154.

What

Mechanical single-source sweep: each duplicated constant now has exactly ONE defining site and one importer. Zero behavior change (grep-verified single sites; identical values/URLs; check/test green). Item 4 dropped per the Batch 22 amendment.

# Duplication Fix (owner → consumers)
1 Bold-face threshold weight>=600?700:400 (render layout.ts + export fonts.ts) new matchFaceWeight in @toony/render; render's measure weight and export cssFaceWeight both call it
2 Per-target default widths (export targets.ts + studio export-panel.tsx) see below (@toony/export/defaults)
3 Encode quality default 82 (export encode.ts + export-panel.tsx hardcode) DEFAULT_JPEG_QUALITY from @toony/export/defaults
4 DROPPED (ComfyUI fallback dims) one-line independence comment at providers/comfyui.ts:39 (request fallback ≠ bundled-workflow defaults)
5 Lint reference render 1200×1600 (craft-lint.ts + overflow-lint.ts) new lint/reference.ts REFERENCE_RENDER; both import it (DEFAULT_OVERFLOW_FALLBACK is now an alias, still exported)
6 FALLBACK_ART (studio project.ts + two episode pages) exported from project.ts; both pages import it
7 coverUrl re-implements assetUrl (studio page.tsx) uses assetUrl, threading workRoot = join(workspaceRoot(), id) from listWorksadds the resolveWorkAsset safety pre-check
8 ComfyUI probe copied (settings page ← /api/config) new lib/comfyui-probe.ts probeComfyui; both call it

Item 2/3 — browser-safe subpath

@toony/export's engine (targets/encode) imports Node-only code (@napi-rs/canvas, node:fs). Importing the plain width/quality constants from the main entry into the client export-panel dragged that engine into the webpack bundle (UnhandledSchemeError: node:fs/promises). Fix: a new Node-free packages/export/src/defaults.ts (just numbers, zero imports) exposed as the @toony/export/defaults subpath (mirrors the @toony/fonts/node pattern). The engine imports these too, and the main entry re-exports them for server/back-compat — so there is exactly one defining site and the client bundle stays engine-free.

Item 8 — intentional SSRF-guard adoption (per amendment)

settings/page.tsx's old probe() LACKED the http/https protocol guard that /api/config's probeConnection() had. The shared probeComfyui includes it, so the settings page now adopts it as a strict security improvement. New test asserts a non-http(s) endpoint (file:/ftp:/gopher:) reports unreachable without a fetch — covering both call paths (they share the helper).

Visible UI

No visible rendering change — every value is byte-identical to before (same default widths/quality, same cover URLs for valid paths). export-panel opens at the same defaults; the works-list covers resolve the same for valid paths (unsafe paths now correctly null, which the server route already blocked). Hence no Design Fidelity table (nothing rendered changed).

EPIC Alignment

  • EPIC EPIC: Build Toony MVP #1 (Build Toony MVP) — drift-proof constants across render/export/lint/studio; a single defining site per value keeps Read=export and config surfaces from silently diverging.
  • Ticket [audit] single-source constants sweep: 8 duplicated values across packages and studio #154 (Batch 22-approved + amendment): the amended 7-item list implemented exactly; item 4 dropped → comment; item 8 adopts the SSRF guard + test; item 7 threads workRoot.
  • Contracts respected: each pair resolves to one exported definition + one importer; settings probe adopts the SSRF guard (tested); coverUrl threads workRoot; zero behavior change elsewhere. Out of scope: item 4 (comment only).

Self-Verification

  • AC — each value one defining site (grep): bold threshold → render/text.ts only; 1200×1600lint/reference.ts only; FALLBACK_ARTproject.ts only; widths/quality → export/defaults.ts only; no probeConnection/coverUrl/hardcoded 82/defaultWidth: 1200 remain (verified). ✅
  • AC — zero behavior change: values identical; @toony/export tests 35 unchanged; studio next build + typecheck clean. ✅
  • AC — pnpm check + pnpm test green: uncached (turbo … --force) on Node 20.20.2 AND 24.15.0check exit 0; 17/17 tasks, 0 fail (@toony/studio 29 → 32: +3 SSRF-guard probe tests). ✅
  • Invariants: no new runtime deps; no stubs; no @toony/render layout-algorithm change.

Out of scope

Item 4 (ComfyUI fallback dims — dropped, comment only); the actual reachable/unreachable-via-fetch probe path (needs a live server; documented, guard path tested).

🤖 Generated with Claude Code

…pped)

Give each duplicated constant exactly one defining site + one importer. Zero
behavior change (grep-verified single sites; check/test green).

1. Bold-face threshold: new `matchFaceWeight` in @toony/render; render's measure
   weight and @toony/export `cssFaceWeight` both call it.
2/3. Export default widths + JPEG quality: new Node-free `@toony/export/defaults`
   (plain constants, no canvas/fs) — the engine (targets/encode) imports them AND
   the studio export-panel single-sources them via the browser-safe subpath
   (importing from the main entry would drag the Node engine into the client
   bundle). Widths/quality re-exported from the main entry too for back-compat.
4. DROPPED (Batch 22): left a one-line independence comment at comfyui.ts:39
   (the request-fallback consts are intentionally separate from the bundled
   workflow JSON's own defaults).
5. Lint reference render 1200×1600: new lint/reference.ts REFERENCE_RENDER;
   craft-lint + overflow-lint (DEFAULT_OVERFLOW_FALLBACK alias) both import it.
6. FALLBACK_ART: exported from studio lib/project.ts; both episode pages import it.
7. Works-list coverUrl: now uses the path-safe `assetUrl`, threading workRoot
   (join(workspaceRoot(), id)) from listWorks — adds the resolveWorkAsset pre-check.
8. ComfyUI probe: new studio lib/comfyui-probe.ts `probeComfyui`; /api/config and
   the settings page both call it. The settings page INTENTIONALLY adopts the
   http/https SSRF protocol guard it previously lacked (strict security
   improvement) — new test asserts a non-http(s) endpoint is unreachable (both
   paths share the helper).

No new runtime deps; no stubs. check + test green uncached Node 20.20.2 + 24.15.0
(studio 29 -> 32; export unchanged).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Epic Alignment: PASS

#154 consolidates the amended drift-prone values at explicit ownership boundaries while preserving behavior except for the required settings SSRF protocol guard.

Checked (evidence)

  • Structural gate: live PR body has filled ## EPIC Alignment and ## Self-Verification; no Design Fidelity table is required because no visible UI rendering changes.
  • Ticket/amendment: live issue #154 confirms item 4 is dropped, item 7 must thread workRoot, and item 8 must adopt/test the http/https guard.
  • Browser boundary: packages/export/src/defaults.ts:1-15 is import-free and exported as @toony/export/defaults; apps/studio/src/components/export-panel.tsx:15-48,91-96 consumes only that subpath.
  • Safety contracts: apps/studio/src/app/page.tsx:25-69 passes join(workspaceRoot(), work.id) to shared assetUrl; apps/studio/src/lib/comfyui-probe.ts:25-58 centralizes the http/https guard, with non-http(s) coverage in lib/__tests__/comfyui-probe.test.ts:17-29.
  • Riskiest part: moving a client default import behind a subpath; the new defaults module has zero Node/canvas imports and leaves engine consumers importing the same constants.
  • Kill-list: scanned changed runtime/test ranges - clean; no dependency, stub, or scope creep. Item 4 is comment-only as required.
  • CI: gh pr checks 171 -> Node 20 PASS (1m34s); Node 24 PASS (1m20s).

Findings

  • None.

Decision

All amended single-source boundaries are represented by a shared definition and consumer imports, with the required security and path-safety improvements tested.

@realproject7

Copy link
Copy Markdown
Owner Author

[@re2 REVIEW — APPROVE] (marked comment; own-identity gh review blocked)

Review of PR #171 (#154), commit e14bbe5 vs origin/main (ba77a20). Verdict: APPROVE / TICKET-OK. Amended 7-item single-source sweep; item 4 dropped.

Checked (evidence)

  • Item 8 — shared probeComfyui + intentional SSRF-guard adoption: config/route.ts drops its ~66-line inline probe and settings/page.tsx its ~44-line one (which LACKED the guard); both now call the single probeComfyui, which short-circuits non-http(s) → unreachable before any fetch. Behavior for the route is preserved; settings gains the guard (documented as intentional). Test asserts file:/ftp:/gopher: → unreachable + unset/malformed branches — covers both call paths via the shared helper. The SSRF test passes by name.
  • Item 7 — coverUrlassetUrl with workRoot: works-list page.tsx now calls assetUrl(work.id, join(workspaceRoot(), work.id), coverImagePath), adding the resolveWorkAsset path-safety pre-check. workRoot derivation matches workspace's join(workspaceRoot(), id); valid covers produce the identical URL, unsafe paths now return null (safety improvement).
  • Items 2/3 — @toony/export/defaults browser-safe subpath: defaults.ts is genuinely Node-free (width/quality consts, no node:/@napi import), exposed via the ./defaults package export and re-exported from the main entry (back-compat). export-panel.tsx (client) now imports the consts from @toony/export/defaults, so the client bundle no longer drags @napi-rs/canvas/node:fs through them. Confirmed real: next build passes.
  • Items 1/5/6 — mechanical hoists (matchFaceWeight in render, REFERENCE_RENDER/DEFAULT_OVERFLOW_FALLBACK alias in lint, FALLBACK_ART export): value-preserving single-sourcing — render 95/0, lint 69/0, export 35/0 all unchanged uncached, so no behavior drift.
  • Item 4 correctly dropped → one-line independence comment at providers/comfyui.ts:39 (no coupling of the fallback consts to the bundled-graph JSON).
  • Reproduced UNCACHED on Node 24.15.0: render 95/0, lint 69/0, export 35/0, @toony/studio 32/0 (29→32), next build + tsc --noEmit green.
  • Invariants: no new deps (export package.json change is the exports subpath, not dependencies); no stubs.

Clean to merge on my side. No further changes.

@realproject7
realproject7 merged commit 238f408 into main Jul 14, 2026
2 checks passed
@realproject7 realproject7 mentioned this pull request Jul 14, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[audit] single-source constants sweep: 8 duplicated values across packages and studio

2 participants