Skip to content

feat: advanced templating with layouts#138

Open
twangodev wants to merge 5 commits intomainfrom
feat/templating-refactor
Open

feat: advanced templating with layouts#138
twangodev wants to merge 5 commits intomainfrom
feat/templating-refactor

Conversation

@twangodev
Copy link
Owner

@twangodev twangodev commented Mar 16, 2026

Summary by CodeRabbit

  • New Features

    • Configurable server endpoint in the Playground.
    • Bulk addition of new gradient themes and composed template support.
    • Layout-based template filtering in the Playground selector.
  • Improvements

    • Centralized image URL generation for consistent thumbnails/previews.
    • Expanded template metadata (colors, fonts, width constraints, layout options).
    • Added frontend "ogis" library dependency to support URL generation.

Copilot AI review requested due to automatic review settings March 16, 2026 01:26
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 16, 2026

📝 Walkthrough

Walkthrough

Adds gradient-based template composition and extensive template-loading refactor in the Rust backend; centralizes frontend image URL generation into a new apiConfig module, introduces a ServerConfig UI, and updates many web components to use the new URL API. Numerous gradient YAML presets and template data updates added.

Changes

Cohort / File(s) Summary
Backend: templates & SVG generator
src/templates.rs, src/generator/svg.rs
Major refactor: new gradient/template composition types and flow (GradientDef, BlobDef, GradientStop, TemplateEntry, TemplateFonts, extended TemplateMap), modular loaders (load_layouts/load_gradients/build_composed_template/etc.), color/gradient assembly utilities; minor change in svg.rs using unwrap_or_default() for width constraints.
Frontend: centralized API config & client
web/src/lib/config/api.svelte.ts, web/package.json
New apiConfig singleton wrapping an OgisClient, persisted baseUrl with normalization/reset, and generateUrl(params) delegating to client; added ogis dependency.
Frontend: components switched to apiConfig.generateUrl
web/src/lib/components/hero/OGCard.svelte, web/src/lib/components/playground/TemplateCard.svelte, web/src/lib/components/sections/HeroContent.svelte, web/src/lib/components/sections/I18nShowcase.svelte, web/src/lib/components/templates/TemplateCard.svelte, web/src/lib/stores/playground.svelte.ts
Replaced manual URLSearchParams/string assembly with calls to apiConfig.generateUrl(...); updated parameter handling and small API surface (TemplateDefinition.layout added).
Frontend: playground server UI & selector changes
web/src/lib/components/playground/ServerConfig.svelte, web/src/routes/playground/+page.svelte, web/src/lib/components/playground/TemplateSelector.svelte
Added ServerConfig component for baseUrl editing/reset; integrated into playground controls; TemplateSelector gained layout filtering (select + counts) and filtering logic updates.
Layouts / runtime template surface
web/src/routes/+layout.server.ts, templates.yaml
Templates YAML restructured: added layouts mapping; templates list simplified to TemplateEntry objects (name/file/colors/layout/gradient); server loader updated to read gradients, generate composed layout×gradient templates, and derive color keys.
Gradient presets (data)
gradients/*.yaml (many new files)
Large set of new gradient configuration YAML files added (e.g., amethyst.yaml, aurora.yaml, tropics.yaml, etc.) defining base_stops, blobs, noise, and text_colors used by the new composition system.

Sequence Diagram(s)

sequenceDiagram
    participant Component as Frontend Component
    participant apiConfig as apiConfig
    participant OgisClient as OgisClient
    participant Server as Image Server

    Component->>apiConfig: generateUrl(params)
    apiConfig->>OgisClient: buildUrl(params)
    OgisClient->>OgisClient: compose baseUrl + params
    OgisClient-->>apiConfig: return URL
    apiConfig-->>Component: return image URL
    Component->>Server: GET image URL
    Server-->>Component: image response
Loading
sequenceDiagram
    participant Loader as Template Loader
    participant Parser as YAML Parser
    participant Compositor as Compositor
    participant Registry as Template Registry

    Loader->>Parser: load_gradients() / load_layouts()
    Parser-->>Loader: parsed GradientDef / layouts
    Loader->>Compositor: build_composed_template(layout, gradient)
    Compositor->>Compositor: build_gradient_defs() / build_layers()
    Compositor-->>Loader: composed SVG template
    Loader->>Registry: register_entry(name, composed_template)
    Registry-->>Loader: templates registered
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰✨ I stitched gradients by moonlight's code,
Composed bright blobs where colors flowed,
A tiny config, stored and neat,
URLs humbly marching to the beat,
Hop! — templates bloom, a rabbit's ode.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: advanced templating with layouts' accurately summarizes the main changes: introducing layout-based template filtering, gradient composition, and modular template loading with extensive API expansion.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/templating-refactor
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 39.85765% with 169 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/templates.rs 39.85% 169 Missing ⚠️
Flag Coverage Δ
rust 25.47% <39.85%> (+3.48%) ⬆️
typescript 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/generator/svg.rs 8.97% <ø> (-1.16%) ⬇️
src/templates.rs 33.92% <39.85%> (+21.51%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a configurable OGIS API base URL to the SvelteKit web app (replacing hardcoded https://img.ogis.dev URL construction), and extends template metadata parsing to support “composed” gradient templates backed by shared SVG layouts.

Changes:

  • Introduces apiConfig (persisted via localStorage) and updates multiple components/stores to generate URLs through it.
  • Adds a “Server” configuration section in the playground UI to edit/reset the API base URL.
  • Updates template YAML parsing/types to support gradient-composed templates and derives their color customization keys.

Reviewed changes

Copilot reviewed 13 out of 99 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
web/src/routes/playground/+page.svelte Adds the new “Server” panel to the playground sidebar.
web/src/routes/+layout.server.ts Extends template YAML parsing to support composed templates + gradient-derived color keys.
web/src/lib/stores/playground.svelte.ts Switches URL generation to use apiConfig.generateUrl(...).
web/src/lib/config/api.svelte.ts New central API base URL config + persistence + URL generation via OgisClient.
web/src/lib/components/templates/TemplateCard.svelte Uses apiConfig for template thumbnail URL generation.
web/src/lib/components/sections/I18nShowcase.svelte Uses apiConfig for showcase image URLs.
web/src/lib/components/sections/HeroContent.svelte Uses apiConfig for hero demo URLs.
web/src/lib/components/playground/TemplateCard.svelte Uses apiConfig for playground template thumbnails.
web/src/lib/components/playground/ServerConfig.svelte New UI to edit/reset the API base URL.
web/src/lib/components/hero/OGCard.svelte Uses apiConfig for hero card image URLs.
web/package.json Adds ogis dependency for URL generation.
templates/layouts/top-bar.svg Adds a reusable composed-template layout SVG.
templates/layouts/stacked.svg Adds a reusable composed-template layout SVG.
templates/layouts/spotlight.svg Adds a reusable composed-template layout SVG.
templates/layouts/split.svg Adds a reusable composed-template layout SVG.
templates/layouts/right-heavy.svg Adds a reusable composed-template layout SVG.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +2
import { OgisClient, type OgisParams } from 'ogis';

Comment on lines +10 to +17
// Load from localStorage on init (browser only)
if (typeof window !== 'undefined') {
const stored = localStorage.getItem(STORAGE_KEY);
if (stored) {
baseUrl = stored;
client = new OgisClient({ baseUrl: stored });
}
}
Comment on lines +23 to +33
set baseUrl(value: string) {
const normalized = value.replace(/\/+$/, '') || DEFAULT_API_URL;
baseUrl = normalized;
client = new OgisClient({ baseUrl: normalized });
if (typeof window !== 'undefined') {
if (normalized === DEFAULT_API_URL) {
localStorage.removeItem(STORAGE_KEY);
} else {
localStorage.setItem(STORAGE_KEY, normalized);
}
}
Comment on lines +10 to +14
function handleInput(e: Event) {
const target = e.target as HTMLInputElement;
inputValue = target.value;
apiConfig.baseUrl = target.value;
}
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (1)
web/src/lib/stores/playground.svelte.ts (1)

123-143: previewUrl may remain stale when only API base URL changes.

buildApiUrl() now uses apiConfig, but debouncedApiUrl is only refreshed from playground-state mutations. Please verify that changing apiConfig.baseUrl alone updates preview URLs immediately.

💡 Suggested reactive hook
 	// Debounced API URL for preview components
 	let debouncedApiUrl = $state('');
 	let apiUrlDebounceTimer: ReturnType<typeof setTimeout> | null = null;
+
+	// Keep preview URL in sync when API server changes
+	$effect(() => {
+		apiConfig.baseUrl;
+		debouncedApiUrl = buildApiUrl();
+	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/lib/stores/playground.svelte.ts` around lines 123 - 143, buildApiUrl
currently reads apiConfig but debouncedApiUrl (the preview URL) only refreshes
when playground-state mutations occur, so changing apiConfig.baseUrl alone
leaves previewUrl stale; update the reactive logic that computes or debounces
the preview URL to also depend on apiConfig.baseUrl (or subscribe to apiConfig)
so that buildApiUrl is re-run when apiConfig.baseUrl changes — locate the code
that derives/debounces previewUrl (references: buildApiUrl, debouncedApiUrl,
playground-state, apiConfig) and include apiConfig.baseUrl as a
trigger/dependency for recomputation or force a subscription callback to update
debouncedApiUrl immediately when apiConfig changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/templates.rs`:
- Around line 616-643: The file-loader silently drops failed SVG loads and
composed templates missing keys; update load_file_template to handle svg load
errors explicitly by matching the result of yaml_loader::load_text instead of
using .ok()?, log a clear tracing::error (including template name and path and
the underlying error) and return None on failure, and likewise update the
composed-template loader (e.g., load_composed_template or the function that
builds TemplateEntry from composed YAML) to validate required keys ("layout" and
"gradient") before constructing TemplateEntry, logging a tracing::error with the
template name and which key is missing and then reject the entry (return None)
so malformed templates are traced instead of disappearing silently.
- Around line 464-480: The compose_template function currently does blind string
replacements and returns a String even when required placeholders are missing;
change compose_template to validate presence of the gradient markers ("<!--
ogis_gradient_defs -->" and "<!-- ogis_background_layers -->") and return an
Option<String> or Result<String, Error> (e.g., None or Err) when they are
absent, so callers can fail fast; update callers that invoke compose_template
(where layout_svg and gradient_def are passed) to handle the new return type by
logging an error like "Layout '{layout_key}' is missing required gradient
placeholders" and dropping the layout instead of using the malformed SVG; keep
build_gradient_defs and build_gradient_layers usage but only call them after
validating the markers exist.
- Around line 265-278: The parse_noise_def function currently casts octave
counts with unchecked `as u32` after using `yaml_int_or`, which allows negative
YAML values to wrap to huge u32s; add a new helper `yaml_u32_or` that reads an
integer (using the same YAML access pattern), attempts `u32::try_from(i64_val)`
and falls back to the provided default on error or out-of-range values, then
replace the two uses of `yaml_int_or(... ) as u32` (for `coarse_octaves` and
`fine_octaves`) with calls to `yaml_u32_or(node, "coarse_octaves",
d.coarse_octaves)` and `yaml_u32_or(node, "fine_octaves", d.fine_octaves)`
respectively so octave counts are validated instead of silently wrapping.

In `@web/src/lib/components/playground/ServerConfig.svelte`:
- Around line 10-19: The handler currently writes normalized/validated
`apiConfig.baseUrl` on every input, causing mid-typing rejections; change
`handleInput` to only update the local `inputValue` (do not touch
`apiConfig.baseUrl`) and add/modify a commit handler (e.g., `handleChange` or
`handleBlur`) that normalizes/validates and sets `apiConfig.baseUrl` once the
edit is complete, then resync `inputValue` from `apiConfig.baseUrl`; also update
`handleReset` to call `apiConfig.reset()` and then set `inputValue =
apiConfig.baseUrl`; apply the same pattern to the other input handlers
referenced around lines 32-38 so only the commit handler persists the URL (or
use a debounced commit if preferred).

In `@web/src/lib/config/api.svelte.ts`:
- Around line 7-16: Replace the manual capture of baseUrl when creating client
with a Svelte derived store so client stays reactive to baseUrl changes: create
client as $derived(baseUrl, $base => new OgisClient({ baseUrl: $base })) (or
using derived(baseUrl, $ => ...)) instead of initializing with the plain $state
value; also apply the same URL normalization logic used in the baseUrl setter
when loading from localStorage (use the normalize function/logic before
assigning baseUrl and before constructing OgisClient) so the init block and the
setter behave consistently; update references to client and baseUrl (OgisClient,
STORAGE_KEY, DEFAULT_API_URL) accordingly and remove manual reassignments that
caused the state_referenced_locally warning.

---

Nitpick comments:
In `@web/src/lib/stores/playground.svelte.ts`:
- Around line 123-143: buildApiUrl currently reads apiConfig but debouncedApiUrl
(the preview URL) only refreshes when playground-state mutations occur, so
changing apiConfig.baseUrl alone leaves previewUrl stale; update the reactive
logic that computes or debounces the preview URL to also depend on
apiConfig.baseUrl (or subscribe to apiConfig) so that buildApiUrl is re-run when
apiConfig.baseUrl changes — locate the code that derives/debounces previewUrl
(references: buildApiUrl, debouncedApiUrl, playground-state, apiConfig) and
include apiConfig.baseUrl as a trigger/dependency for recomputation or force a
subscription callback to update debouncedApiUrl immediately when apiConfig
changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 31925266-590e-4fd4-81fd-2e44d83973fd

📥 Commits

Reviewing files that changed from the base of the PR and between abb16e0 and 5a2ac03.

⛔ Files ignored due to path filters (85)
  • package-lock.json is excluded by !**/package-lock.json
  • templates/gradient-amethyst.svg is excluded by !**/*.svg
  • templates/gradient-arctic.svg is excluded by !**/*.svg
  • templates/gradient-aurora.svg is excluded by !**/*.svg
  • templates/gradient-autumn.svg is excluded by !**/*.svg
  • templates/gradient-berry.svg is excluded by !**/*.svg
  • templates/gradient-candy.svg is excluded by !**/*.svg
  • templates/gradient-cedar.svg is excluded by !**/*.svg
  • templates/gradient-champagne.svg is excluded by !**/*.svg
  • templates/gradient-citrus.svg is excluded by !**/*.svg
  • templates/gradient-cloud.svg is excluded by !**/*.svg
  • templates/gradient-cobalt.svg is excluded by !**/*.svg
  • templates/gradient-copper-teal.svg is excluded by !**/*.svg
  • templates/gradient-copper.svg is excluded by !**/*.svg
  • templates/gradient-coral-navy.svg is excluded by !**/*.svg
  • templates/gradient-coral.svg is excluded by !**/*.svg
  • templates/gradient-cosmos.svg is excluded by !**/*.svg
  • templates/gradient-dawn.svg is excluded by !**/*.svg
  • templates/gradient-duotone.svg is excluded by !**/*.svg
  • templates/gradient-dusk.svg is excluded by !**/*.svg
  • templates/gradient-eclipse.svg is excluded by !**/*.svg
  • templates/gradient-ember-ice.svg is excluded by !**/*.svg
  • templates/gradient-ember.svg is excluded by !**/*.svg
  • templates/gradient-evergreen.svg is excluded by !**/*.svg
  • templates/gradient-fig.svg is excluded by !**/*.svg
  • templates/gradient-forest.svg is excluded by !**/*.svg
  • templates/gradient-galaxy.svg is excluded by !**/*.svg
  • templates/gradient-garnet.svg is excluded by !**/*.svg
  • templates/gradient-ginger.svg is excluded by !**/*.svg
  • templates/gradient-hazel.svg is excluded by !**/*.svg
  • templates/gradient-heather.svg is excluded by !**/*.svg
  • templates/gradient-horizon.svg is excluded by !**/*.svg
  • templates/gradient-indigo-blush.svg is excluded by !**/*.svg
  • templates/gradient-jade.svg is excluded by !**/*.svg
  • templates/gradient-lagoon.svg is excluded by !**/*.svg
  • templates/gradient-lavender.svg is excluded by !**/*.svg
  • templates/gradient-lilac.svg is excluded by !**/*.svg
  • templates/gradient-lotus.svg is excluded by !**/*.svg
  • templates/gradient-maple.svg is excluded by !**/*.svg
  • templates/gradient-mauve.svg is excluded by !**/*.svg
  • templates/gradient-midnight.svg is excluded by !**/*.svg
  • templates/gradient-mint.svg is excluded by !**/*.svg
  • templates/gradient-moonstone.svg is excluded by !**/*.svg
  • templates/gradient-moss.svg is excluded by !**/*.svg
  • templates/gradient-neon.svg is excluded by !**/*.svg
  • templates/gradient-noir.svg is excluded by !**/*.svg
  • templates/gradient-noire.svg is excluded by !**/*.svg
  • templates/gradient-ocean.svg is excluded by !**/*.svg
  • templates/gradient-opal.svg is excluded by !**/*.svg
  • templates/gradient-orchid.svg is excluded by !**/*.svg
  • templates/gradient-peach-lavender.svg is excluded by !**/*.svg
  • templates/gradient-peach.svg is excluded by !**/*.svg
  • templates/gradient-pine.svg is excluded by !**/*.svg
  • templates/gradient-plum-gold.svg is excluded by !**/*.svg
  • templates/gradient-quartz.svg is excluded by !**/*.svg
  • templates/gradient-raven.svg is excluded by !**/*.svg
  • templates/gradient-ruby.svg is excluded by !**/*.svg
  • templates/gradient-saffron.svg is excluded by !**/*.svg
  • templates/gradient-sage-terracotta.svg is excluded by !**/*.svg
  • templates/gradient-sahara.svg is excluded by !**/*.svg
  • templates/gradient-sapphire.svg is excluded by !**/*.svg
  • templates/gradient-seaglass.svg is excluded by !**/*.svg
  • templates/gradient-shadow.svg is excluded by !**/*.svg
  • templates/gradient-sherbet.svg is excluded by !**/*.svg
  • templates/gradient-steel.svg is excluded by !**/*.svg
  • templates/gradient-storm.svg is excluded by !**/*.svg
  • templates/gradient-sunset.svg is excluded by !**/*.svg
  • templates/gradient-terra.svg is excluded by !**/*.svg
  • templates/gradient-thunderstorm.svg is excluded by !**/*.svg
  • templates/gradient-tropics.svg is excluded by !**/*.svg
  • templates/gradient-velvet.svg is excluded by !**/*.svg
  • templates/gradient-voltage.svg is excluded by !**/*.svg
  • templates/gradient-wine.svg is excluded by !**/*.svg
  • templates/layouts/banner.svg is excluded by !**/*.svg
  • templates/layouts/bottom-bar.svg is excluded by !**/*.svg
  • templates/layouts/centered.svg is excluded by !**/*.svg
  • templates/layouts/editorial.svg is excluded by !**/*.svg
  • templates/layouts/left-heavy.svg is excluded by !**/*.svg
  • templates/layouts/minimal.svg is excluded by !**/*.svg
  • templates/layouts/reverse-split.svg is excluded by !**/*.svg
  • templates/layouts/right-heavy.svg is excluded by !**/*.svg
  • templates/layouts/split.svg is excluded by !**/*.svg
  • templates/layouts/spotlight.svg is excluded by !**/*.svg
  • templates/layouts/stacked.svg is excluded by !**/*.svg
  • templates/layouts/top-bar.svg is excluded by !**/*.svg
📒 Files selected for processing (14)
  • src/generator/svg.rs
  • src/templates.rs
  • templates.yaml
  • web/package.json
  • web/src/lib/components/hero/OGCard.svelte
  • web/src/lib/components/playground/ServerConfig.svelte
  • web/src/lib/components/playground/TemplateCard.svelte
  • web/src/lib/components/sections/HeroContent.svelte
  • web/src/lib/components/sections/I18nShowcase.svelte
  • web/src/lib/components/templates/TemplateCard.svelte
  • web/src/lib/config/api.svelte.ts
  • web/src/lib/stores/playground.svelte.ts
  • web/src/routes/+layout.server.ts
  • web/src/routes/playground/+page.svelte

Comment on lines +265 to +278
fn parse_noise_def(node: &Yaml) -> NoiseDef {
let d = NoiseDef::default();
NoiseDef {
coarse_opacity: yaml_num_or(node, "coarse_opacity", d.coarse_opacity),
fine_opacity: yaml_num_or(node, "fine_opacity", d.fine_opacity),
coarse_frequency: yaml_num_or(node, "coarse_frequency", d.coarse_frequency),
coarse_octaves: yaml_int_or(node, "coarse_octaves", d.coarse_octaves as i64) as u32,
coarse_slope: yaml_num_or(node, "coarse_slope", d.coarse_slope),
coarse_intercept: yaml_num_or(node, "coarse_intercept", d.coarse_intercept),
fine_frequency: yaml_num_or(node, "fine_frequency", d.fine_frequency),
fine_octaves: yaml_int_or(node, "fine_octaves", d.fine_octaves as i64) as u32,
fine_slope: yaml_num_or(node, "fine_slope", d.fine_slope),
fine_intercept: yaml_num_or(node, "fine_intercept", d.fine_intercept),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In current Rust, what does (-1_i64) as u32evaluate to, and are signed-to-unsigned integer casts withas range-checked?

💡 Result:

(-1_i64) as u32 evaluates to 4294967295u32 (i.e., 0xFFFF_FFFF). This happens because Rust integer casts with as truncate to the target bit-width and, for same-width signed/unsigned casts, are effectively a bit reinterpretation under two’s complement. They are not range-checked (no panic/error); out-of-range values silently wrap/truncate. [1]


🏁 Script executed:

# Verify the file exists and check line count
wc -l src/templates.rs

Repository: twangodev/ogis

Length of output: 78


🏁 Script executed:

# Read the relevant section around lines 265-278
sed -n '260,285p' src/templates.rs

Repository: twangodev/ogis

Length of output: 1310


🏁 Script executed:

# Search for existing yaml_*_or helper functions to understand the pattern
rg 'fn yaml_.*_or\(' src/templates.rs -A 5

Repository: twangodev/ogis

Length of output: 519


🏁 Script executed:

# Find NoiseDef struct definition
rg 'struct NoiseDef' -A 20

Repository: twangodev/ogis

Length of output: 912


🏁 Script executed:

# Search for other instances of yaml_int_or followed by as u32 or as i32
rg 'yaml_int_or.*as [ui]32' src/templates.rs

Repository: twangodev/ogis

Length of output: 237


🏁 Script executed:

# Check if there are any other unchecked integer casts related to yaml parsing
rg 'yaml.*as [ui](?:8|16|32|64)' src/templates.rs

Repository: twangodev/ogis

Length of output: 237


Validate octave counts before casting to prevent silent wrapping of negative values.

Lines 271 and 275 use unchecked as u32 casts on i64 values. In Rust, (-1_i64) as u32 silently evaluates to 4294967295 without range validation. A negative octave value in YAML will wrap to a massive u32, injecting absurd numOctaves values into the generated filter.

Add a yaml_u32_or helper that validates the range using u32::try_from():

Proposed fix
+fn yaml_u32_or(node: &Yaml, key: &str, default: u32) -> u32 {
+    yaml_get(node, key)
+        .and_then(|v| v.as_integer())
+        .and_then(|i| u32::try_from(i).ok())
+        .unwrap_or(default)
+}
+
 fn parse_noise_def(node: &Yaml) -> NoiseDef {
     let d = NoiseDef::default();
     NoiseDef {
         coarse_opacity: yaml_num_or(node, "coarse_opacity", d.coarse_opacity),
         fine_opacity: yaml_num_or(node, "fine_opacity", d.fine_opacity),
         coarse_frequency: yaml_num_or(node, "coarse_frequency", d.coarse_frequency),
-        coarse_octaves: yaml_int_or(node, "coarse_octaves", d.coarse_octaves as i64) as u32,
+        coarse_octaves: yaml_u32_or(node, "coarse_octaves", d.coarse_octaves),
         coarse_slope: yaml_num_or(node, "coarse_slope", d.coarse_slope),
         coarse_intercept: yaml_num_or(node, "coarse_intercept", d.coarse_intercept),
         fine_frequency: yaml_num_or(node, "fine_frequency", d.fine_frequency),
-        fine_octaves: yaml_int_or(node, "fine_octaves", d.fine_octaves as i64) as u32,
+        fine_octaves: yaml_u32_or(node, "fine_octaves", d.fine_octaves),
         fine_slope: yaml_num_or(node, "fine_slope", d.fine_slope),
         fine_intercept: yaml_num_or(node, "fine_intercept", d.fine_intercept),
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/templates.rs` around lines 265 - 278, The parse_noise_def function
currently casts octave counts with unchecked `as u32` after using `yaml_int_or`,
which allows negative YAML values to wrap to huge u32s; add a new helper
`yaml_u32_or` that reads an integer (using the same YAML access pattern),
attempts `u32::try_from(i64_val)` and falls back to the provided default on
error or out-of-range values, then replace the two uses of `yaml_int_or(... ) as
u32` (for `coarse_octaves` and `fine_octaves`) with calls to `yaml_u32_or(node,
"coarse_octaves", d.coarse_octaves)` and `yaml_u32_or(node, "fine_octaves",
d.fine_octaves)` respectively so octave counts are validated instead of silently
wrapping.

Comment on lines +464 to +480
fn compose_template(layout_svg: &str, gradient: &GradientDef) -> String {
let tc = &gradient.text_colors;
layout_svg
.replace(
"<!-- ogis_gradient_defs -->",
&build_gradient_defs(gradient),
)
.replace(
"<!-- ogis_background_layers -->",
&build_gradient_layers(gradient),
)
.replace("{{title_color}}", &tc.title)
.replace("{{desc_color}}", &tc.description)
.replace("{{subtitle_color}}", &tc.subtitle)
.replace("{{desc_opacity}}", &fmt_num(tc.desc_opacity))
.replace("{{subtitle_opacity}}", &fmt_num(tc.subtitle_opacity))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fail fast when a layout omits the gradient markers.

Lines 467-474 are blind replacements, so a typo in <!-- ogis_gradient_defs --> or <!-- ogis_background_layers --> quietly produces a half-composed template with no injected defs/background. This should abort registration instead of shipping a broken SVG.

🧩 Proposed fix
-fn compose_template(layout_svg: &str, gradient: &GradientDef) -> String {
+fn compose_template(layout_svg: &str, gradient: &GradientDef) -> Option<String> {
+    if !layout_svg.contains("<!-- ogis_gradient_defs -->")
+        || !layout_svg.contains("<!-- ogis_background_layers -->")
+    {
+        return None;
+    }
+
     let tc = &gradient.text_colors;
-    layout_svg
+    Some(layout_svg
         .replace(
             "<!-- ogis_gradient_defs -->",
             &build_gradient_defs(gradient),
         )
         .replace(
             "<!-- ogis_background_layers -->",
             &build_gradient_layers(gradient),
         )
         .replace("{{title_color}}", &tc.title)
         .replace("{{desc_color}}", &tc.description)
         .replace("{{subtitle_color}}", &tc.subtitle)
         .replace("{{desc_opacity}}", &fmt_num(tc.desc_opacity))
-        .replace("{{subtitle_opacity}}", &fmt_num(tc.subtitle_opacity))
+        .replace("{{subtitle_opacity}}", &fmt_num(tc.subtitle_opacity)))
 }

Update the caller to log and drop invalid layouts:

let svg = compose_template(layout_svg, gradient_def).or_else(|| {
    tracing::error!("Layout '{layout_key}' is missing required gradient placeholders");
    None
})?;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/templates.rs` around lines 464 - 480, The compose_template function
currently does blind string replacements and returns a String even when required
placeholders are missing; change compose_template to validate presence of the
gradient markers ("<!-- ogis_gradient_defs -->" and "<!-- ogis_background_layers
-->") and return an Option<String> or Result<String, Error> (e.g., None or Err)
when they are absent, so callers can fail fast; update callers that invoke
compose_template (where layout_svg and gradient_def are passed) to handle the
new return type by logging an error like "Layout '{layout_key}' is missing
required gradient placeholders" and dropping the layout instead of using the
malformed SVG; keep build_gradient_defs and build_gradient_layers usage but only
call them after validating the markers exist.

Comment on lines +616 to +643
fn load_file_template(node: &Yaml) -> Option<TemplateEntry> {
let name = yaml_str(node, "name")?;
let path = yaml_str(node, "file")?;
let svg = yaml_loader::load_text(&path, &format!("template '{name}'")).ok()?;

let fonts = parse_font_properties(&svg);
tracing::info!(
"Loaded '{name}' from {path}: title={}px/w{}, desc={}px/w{}, subtitle={}px/w{}",
fonts.title.size,
fonts.title.weight,
fonts.description.size,
fonts.description.weight,
fonts.subtitle.size,
fonts.subtitle.weight,
);

let colors = parse_template_colors(node);

Some(TemplateEntry {
name,
svg,
colors,
fonts,
width_constraints: parse_width_constraints(node),
truncation: yaml_bool_or(node, "truncation", true),
max_scale: yaml_num_or(node, "max_scale", 1.0),
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Log and reject malformed template entries explicitly.

Line 619 swallows file-load errors with .ok()?, and Line 732 routes any entry missing either layout or gradient into that silent fallback. A misspelled composed-template key can therefore make the template disappear with no tracing.

🪵 Proposed fix
 fn load_file_template(node: &Yaml) -> Option<TemplateEntry> {
     let name = yaml_str(node, "name")?;
     let path = yaml_str(node, "file")?;
-    let svg = yaml_loader::load_text(&path, &format!("template '{name}'")).ok()?;
+    let svg = match yaml_loader::load_text(&path, &format!("template '{name}'")) {
+        Ok(svg) => svg,
+        Err(e) => {
+            tracing::error!("Failed to load template '{name}' from {path}: {e}");
+            return None;
+        }
+    };
             let entry = match (
                 yaml_str(node, "name"),
                 yaml_str(node, "layout"),
                 yaml_str(node, "gradient"),
             ) {
                 (Some(name), Some(layout), Some(gradient)) => {
                     build_composed_template(&name, &layout, &gradient, node, &layouts, &gradients)
                 }
+                (Some(name), Some(_), None) | (Some(name), None, Some(_)) => {
+                    tracing::error!(
+                        "Template '{name}' must define both 'layout' and 'gradient' for composed loading"
+                    );
+                    None
+                }
                 _ => load_file_template(node),
             };

Also applies to: 722-733

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/templates.rs` around lines 616 - 643, The file-loader silently drops
failed SVG loads and composed templates missing keys; update load_file_template
to handle svg load errors explicitly by matching the result of
yaml_loader::load_text instead of using .ok()?, log a clear tracing::error
(including template name and path and the underlying error) and return None on
failure, and likewise update the composed-template loader (e.g.,
load_composed_template or the function that builds TemplateEntry from composed
YAML) to validate required keys ("layout" and "gradient") before constructing
TemplateEntry, logging a tracing::error with the template name and which key is
missing and then reject the entry (return None) so malformed templates are
traced instead of disappearing silently.

Comment on lines +10 to +19
function handleInput(e: Event) {
const target = e.target as HTMLInputElement;
inputValue = target.value;
apiConfig.baseUrl = target.value;
}

function handleReset() {
apiConfig.reset();
inputValue = apiConfig.baseUrl;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Only persist the server URL once the edit is complete.

Line 13 normalizes, validates, and persists on every input event, while Line 12 keeps the raw textbox value. That makes partial edits brittle and expensive, and if the setter rejects a temporary invalid URL the field will fail mid-typing. Keep inputValue local during oninput, then commit and resync from apiConfig.baseUrl on change/blur or after a debounce.

💡 Proposed fix
 function handleInput(e: Event) {
 	const target = e.target as HTMLInputElement;
 	inputValue = target.value;
-	apiConfig.baseUrl = target.value;
+}
+
+function commitInput() {
+	apiConfig.baseUrl = inputValue;
+	inputValue = apiConfig.baseUrl;
 }
 ...
 	value={inputValue}
 	oninput={handleInput}
+	onchange={commitInput}
 />

Also applies to: 32-38

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/lib/components/playground/ServerConfig.svelte` around lines 10 - 19,
The handler currently writes normalized/validated `apiConfig.baseUrl` on every
input, causing mid-typing rejections; change `handleInput` to only update the
local `inputValue` (do not touch `apiConfig.baseUrl`) and add/modify a commit
handler (e.g., `handleChange` or `handleBlur`) that normalizes/validates and
sets `apiConfig.baseUrl` once the edit is complete, then resync `inputValue`
from `apiConfig.baseUrl`; also update `handleReset` to call `apiConfig.reset()`
and then set `inputValue = apiConfig.baseUrl`; apply the same pattern to the
other input handlers referenced around lines 32-38 so only the commit handler
persists the URL (or use a debounced commit if preferred).

Comment on lines +7 to +16
let baseUrl = $state(DEFAULT_API_URL);
let client = $state(new OgisClient({ baseUrl }));

// Load from localStorage on init (browser only)
if (typeof window !== 'undefined') {
const stored = localStorage.getItem(STORAGE_KEY);
if (stored) {
baseUrl = stored;
client = new OgisClient({ baseUrl: stored });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n web/src/lib/config/api.svelte.ts

Repository: twangodev/ogis

Length of output: 1868


🌐 Web query:

Svelte 5 $derived reactive capture warning $state

💡 Result:

In Svelte 5, the “reactive capture” warning you’re seeing is almost always state_referenced_locally:

“This reference only captures the initial value of X…” — meaning you read a $state value in a place that won’t re-run when that state later changes/reassigns, so you effectively “freeze” the initial value. [1]

Common triggers:

  • Passing $state by value (e.g. setContext('count', count)) — the receiver gets a snapshot-like value that won’t update after count changes. [1]
  • Destructuring / extracting from something reactive (props/context/state object) into locals without keeping it derived (the destructure runs once). A typical fix is to destructure from a $derived(...) result instead. [2]

Fix patterns:

  1. Use a lazy reference (closure)
setContext('count', () => count);
// child: const count = getContext('count');  count()

This is the fix the Svelte warning docs recommend. [1]

  1. Use $derived when you want a reactive “view”
const userId = $derived(data.userId);

This keeps userId tied to changes in the reactive source (instead of capturing the initial value). [2]

  1. If you intentionally want the initial value, take an explicit snapshot (so the code documents intent), e.g. with $state.snapshot(...) (often used to silence the warning when the “capture” is desired). [3]

Sources: Svelte compiler warning docs for state_referenced_locally [1], destructuring/reactivity discussion and $derived fix example [2], $state.snapshot approach write-up [3].

[1] (svelte.dev)
[2] (stackoverflow.com)
[3] (mepnnams.com)

Citations:


🌐 Web query:

Svelte 5 $derived vs $state reactive value rune

💡 Result:

$state (source, mutable reactive value)

Use $state(...) for state you intend to change (reassign or mutate). Primitives behave like normal values; objects/arrays become deeply reactive proxies, so mutations like obj.x++ or arr.push() trigger granular updates. If you don’t want deep proxying, use $state.raw(...) (must reassign, no mutation). You can also take a non-proxy copy via $state.snapshot(...). [2]

<script>
  let count = $state(0);
  let todos = $state([{ text: 'Learn', done: false }]);
</script>

$derived (computed reactive value)

Use $derived(...) for a value that is computed from other reactive values and should stay in sync automatically. The expression is evaluated in a reactive context: anything read synchronously inside it becomes a dependency. The expression should be side‑effect free (Svelte disallows state changes inside derived expressions). For more complex computations, use $derived.by(() => { ... }). $derived values are left as-is (not deep-proxied like $state). [1]

<script>
  let count = $state(0);
  let doubled = $derived(count * 2);

  let total = $derived.by(() => {
    // complex calculation
    return count * 2;
  });
</script>

Practical rule of thumb

  • I will set/mutate it$state
  • It’s purely calculated from other state/props$derived (or $derived.by)
  • It’s a side effect (logging, DOM, fetch, etc.)$effect (not $derived) [1]

Common “why didn’t it update?” gotcha

If you capture a $state value in the same scope (e.g., pass count somewhere) and then later reassign count, you can “break the link” and the captured value won’t update—Svelte warns state_referenced_locally. Wrap it in a function to keep it live (() => count). [3]

Sources: Svelte docs for $derived [1], $state [2], and compiler warning state_referenced_locally [3].


Resolve the Svelte reactive-capture warning and normalize URLs consistently.

The current client initialization triggers the state_referenced_locally warning by capturing only the initial baseUrl. Using $derived keeps client automatically synchronized with baseUrl changes and eliminates manual reassignments. Additionally, the init block currently skips URL normalization, unlike the setter—the fix applies consistent normalization everywhere.

♻️ Proposed fix
 function createApiConfig() {
 	let baseUrl = $state(DEFAULT_API_URL);
-	let client = $state(new OgisClient({ baseUrl }));
+	const client = $derived(new OgisClient({ baseUrl }));

 	// Load from localStorage on init (browser only)
 	if (typeof window !== 'undefined') {
 		const stored = localStorage.getItem(STORAGE_KEY);
 		if (stored) {
-			baseUrl = stored;
-			client = new OgisClient({ baseUrl: stored });
+			baseUrl = stored.replace(/\/+$/, '') || DEFAULT_API_URL;
 		}
 	}

 	return {
 		get baseUrl() {
 			return baseUrl;
 		},
 		set baseUrl(value: string) {
 			const normalized = value.replace(/\/+$/, '') || DEFAULT_API_URL;
 			baseUrl = normalized;
-			client = new OgisClient({ baseUrl: normalized });
 			if (typeof window !== 'undefined') {
 				if (normalized === DEFAULT_API_URL) {
 					localStorage.removeItem(STORAGE_KEY);
 				} else {
 					localStorage.setItem(STORAGE_KEY, normalized);
 				}
 			}
 		},
@@
 		reset() {
 			baseUrl = DEFAULT_API_URL;
-			client = new OgisClient({ baseUrl: DEFAULT_API_URL });
 			if (typeof window !== 'undefined') {
 				localStorage.removeItem(STORAGE_KEY);
 			}
 		},

Also applies to: 23-27, 41-44

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/lib/config/api.svelte.ts` around lines 7 - 16, Replace the manual
capture of baseUrl when creating client with a Svelte derived store so client
stays reactive to baseUrl changes: create client as $derived(baseUrl, $base =>
new OgisClient({ baseUrl: $base })) (or using derived(baseUrl, $ => ...))
instead of initializing with the plain $state value; also apply the same URL
normalization logic used in the baseUrl setter when loading from localStorage
(use the normalize function/logic before assigning baseUrl and before
constructing OgisClient) so the init block and the setter behave consistently;
update references to client and baseUrl (OgisClient, STORAGE_KEY,
DEFAULT_API_URL) accordingly and remove manual reassignments that caused the
state_referenced_locally warning.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (2)
src/templates.rs (2)

464-480: ⚠️ Potential issue | 🟠 Major

Still unresolved: fail fast when layout gradient markers are missing.

This still performs blind replacements; malformed layouts can produce partially composed SVGs silently instead of being rejected.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/templates.rs` around lines 464 - 480, compose_template currently performs
blind replacements and should fail fast when expected gradient markers or
placeholders are missing: before calling
build_gradient_defs/build_gradient_layers or performing .replace, assert that
layout_svg.contains("<!-- ogis_gradient_defs -->"), layout_svg.contains("<!--
ogis_background_layers -->") and contains all placeholders "{{title_color}}",
"{{desc_color}}", "{{subtitle_color}}", "{{desc_opacity}}",
"{{subtitle_opacity}}"; if any are absent, return an explicit error (change
compose_template to return Result<String, Error> or panic with a clear message)
so malformed layouts are rejected rather than silently producing partial SVGs;
keep calls to build_gradient_defs and build_gradient_layers and the same
replacements after the checks.

628-632: ⚠️ Potential issue | 🟠 Major

Still unresolved: file-template load errors are silently dropped.

Using .ok()? on template SVG load continues to hide actionable failures and makes entries disappear without explicit error context.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/templates.rs` around lines 628 - 632, In load_file_template, the call to
yaml_loader::load_text(&path, &format!("template '{name}'")).ok()? silently
swallows errors; change this to explicitly handle Err by matching the Result,
log or return the error details (for example using tracing::error! or eprintln!
with path, name and the error) before returning None, so failures to load the
SVG are visible; locate the match in fn load_file_template where yaml_str,
yaml_loader::load_text and TemplateEntry are used and replace the .ok()? chain
with a match that logs the Err variant (including the error message) and
continues to return None or propagate an error if you prefer changing the
function to return Result.
🧹 Nitpick comments (6)
gradients/midnight.yaml (1)

18-19: Set indigo stop opacity explicitly.

This stop currently depends on implicit default opacity; explicit declaration improves clarity and consistency with neighboring blobs.

Suggested diff
-  - {offset: 0, color: '#3730a3'}
+  - {offset: 0, color: '#3730a3', opacity: 1.0}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/midnight.yaml` around lines 18 - 19, The gradient stop with color
'#3730a3' at offset 0 is missing an explicit opacity; update the stop object for
offset: 0 (color: '#3730a3') to include an explicit opacity value (e.g.,
opacity: 1.0) so it no longer relies on implicit defaults and matches the
explicit opacity style used for the offset: 100 stop.
gradients/moss.yaml (1)

18-19: Make lime opacity explicit for consistency.

The first lime stop relies on implicit opacity, unlike most other blob stops. Explicitly setting it avoids renderer-default coupling.

Suggested diff
-  - {offset: 0, color: '#65a30d'}
+  - {offset: 0, color: '#65a30d', opacity: 1.0}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/moss.yaml` around lines 18 - 19, The two lime gradient stops
include a trailing stop with opacity but the leading stop at {offset: 0, color:
'#65a30d'} omits opacity; update that first stop in moss.yaml to include an
explicit opacity (e.g., opacity: 1.0) so both lime stops declare opacity
consistently and avoid relying on renderer defaults.
gradients/terra.yaml (1)

27-35: Prefer semantic blob names over ordinal placeholders.

color_4 / color_5 are harder to maintain than descriptive names (especially when debugging or iterating palettes).

Example naming cleanup
-- name: color_4
+- name: clay
...
-- name: color_5
+- name: tan
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/terra.yaml` around lines 27 - 35, Replace the ordinal blob names
color_4 and color_5 with descriptive, semantic identifiers (e.g.,
terra-warm-center, terra-warm-fade or names that describe role/position like
warm-center and warm-edge) in the YAML so intent is clear; update every
reference to those keys elsewhere in the repo or asset pipeline to match the new
names and preserve the existing properties (cx, cy, r, stops) exactly so
rendering is unchanged.
gradients/hazel.yaml (1)

27-40: Consider using descriptive blob names for consistency.

Blobs color_4 and color_5 use generic names while the others (gold, brown, olive) use descriptive color names. For maintainability and consistency with other gradient files, consider renaming these to descriptive names like lime and beige based on their actual colors (#9acd32 is yellow-green, #c9b896 is a warm beige).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/hazel.yaml` around lines 27 - 40, Rename the generic blob entries
color_4 and color_5 to descriptive names reflecting their colors (e.g., change
color_4 -> lime and color_5 -> beige) in the gradients definition, update their
corresponding blocks (cx, cy, r, stops) to use the new names, and ensure any
code or templates that reference color_4 or color_5 are updated to the new
identifiers (search for uses of color_4 and color_5 and replace them with lime
and beige to maintain consistency with other blobs like gold, brown, olive).
gradients/amethyst.yaml (1)

1-44: Add CI schema validation for gradients/*.yaml.

Given runtime auto-discovery in src/templates.rs, a malformed YAML can silently reduce available templates. A lightweight schema/lint check in CI would catch key/typing/range regressions before merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/amethyst.yaml` around lines 1 - 44, The gradients YAML files
(gradients/*.yaml) can be malformed and silently reduce templates at runtime;
add a lightweight CI job that validates all gradients/*.yaml before merge by
running a schema/lint step (e.g., yamllint or a YAML→JSON Schema/validator like
yamale/ajv) against a small schema that enforces required top-level keys
(base_stops, direction, blobs, noise, text_colors), types (arrays for
base_stops/blobs/stops, numbers for cx/cy/r/offsets, strings for color, numeric
opacities between 0–1), and required blob stop fields (name, cx, cy, r, stops
with offset/color). Wire the job into CI to fail on any schema or type/range
errors so malformed YAMLs are caught before src/templates.rs auto-discovery
runs.
web/src/routes/+layout.server.ts (1)

30-63: Avoid recomputing template composition on every load invocation.

The sync reads/parsing/composition in the load path are avoidable overhead; cache the computed template metadata once at module scope (or memoize) and reuse it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/routes/`+layout.server.ts around lines 30 - 63, The current load()
function recomputes repoRoot, reads/parses templates.yaml and gradients, and
rebuilds composedTemplates/allTemplates on every invocation; move that one-time
file I/O and composition to module scope or implement a module-scoped memo
(e.g., a cachedTemplates or initTemplates() with a boolean/Promise) so the YAML
parsing and gradient directory reads run only once at import time, then have
load() return the cached composedTemplates/allTemplates; reference the existing
symbols load, repoRoot, templates.yaml parsing, gradientDefs, composedTemplates
and allTemplates when relocating the logic and ensure load still returns the
same structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gradients/copper.yaml`:
- Line 18: The orange stop in copper.yaml (offset: 0, color: '#d97706') and the
crimson stop in berry.yaml omit an explicit opacity while other offset-0 stops
include it; to standardize, add an explicit opacity: 1.0 to those offset: 0
stops (e.g., the orange stop in copper.yaml and the crimson stop in berry.yaml)
so all offset-0 stops explicitly declare opacity instead of relying on the SVG
default.

In `@gradients/lavender.yaml`:
- Around line 13-19: The preset "light_violet" defines gradient stops where the
final stop (stops entry with offset: 100) omits an explicit opacity; update that
stop to include opacity: 0.0 so all leading stops explicitly declare opacity,
ensuring consistent rendering irrespective of defaults (look for the "name:
light_violet" block and its "stops" entries, especially the stop with offset
100).

In `@gradients/peach.yaml`:
- Around line 13-19: The preset "light_peach" has a leading gradient stop
without an explicit opacity (under the stops list for name: light_peach); update
the first stop (offset: 0, color: '#fed7aa') to include an explicit opacity
value (e.g., opacity: 1.0) so the gradient does not rely on parser defaults.

In `@src/templates.rs`:
- Around line 723-735: The template generator now registers template names as
"gradient-{gradient}-{layout}" (see template_name variable and
build_composed_template/register_entry), but CardStack.svelte and
I18nShowcase.svelte still reference old "gradient-{gradient}" names causing
not-found errors; update those files to either (A) change each template
reference to include a layout suffix (use one of the available layouts:
left-heavy, centered, split, minimal, right-heavy, stacked, banner, editorial,
spotlight, reverse-split, top-bar, bottom-bar) for at least 18 occurrences, e.g.
"gradient-galaxy-centered", or (B) implement a fallback lookup function in those
components that, given a gradient name, first attempts to use
"gradient-{gradient}-{layout}" (try a preferred layout like "centered" or
iterate layouts) and only then falls back to older "gradient-{gradient}" if
needed; update references to use that lookup helper so all template resolutions
match the names produced by build_composed_template/register_entry.

In `@web/src/routes/`+layout.server.ts:
- Around line 33-47: The current layout load reads and parses templates.yaml and
iterates gradientsDir with readFileSync/readdirSync/parse without guards, so
missing files or malformed YAML will throw and 500 the app; wrap the
templates.yaml load (yamlPath, readFileSync, parse) in a try/catch and on
failure log the error and fall back to a safe default TemplatesYaml, and
similarly check that gradientsDir exists before readdirSync and wrap each
gradient file read/parse in a try/catch so invalid files are skipped (log the
filename + error) while leaving gradientDefs (the gradientDefs object) as an
empty or partial map rather than letting exceptions bubble.

---

Duplicate comments:
In `@src/templates.rs`:
- Around line 464-480: compose_template currently performs blind replacements
and should fail fast when expected gradient markers or placeholders are missing:
before calling build_gradient_defs/build_gradient_layers or performing .replace,
assert that layout_svg.contains("<!-- ogis_gradient_defs -->"),
layout_svg.contains("<!-- ogis_background_layers -->") and contains all
placeholders "{{title_color}}", "{{desc_color}}", "{{subtitle_color}}",
"{{desc_opacity}}", "{{subtitle_opacity}}"; if any are absent, return an
explicit error (change compose_template to return Result<String, Error> or panic
with a clear message) so malformed layouts are rejected rather than silently
producing partial SVGs; keep calls to build_gradient_defs and
build_gradient_layers and the same replacements after the checks.
- Around line 628-632: In load_file_template, the call to
yaml_loader::load_text(&path, &format!("template '{name}'")).ok()? silently
swallows errors; change this to explicitly handle Err by matching the Result,
log or return the error details (for example using tracing::error! or eprintln!
with path, name and the error) before returning None, so failures to load the
SVG are visible; locate the match in fn load_file_template where yaml_str,
yaml_loader::load_text and TemplateEntry are used and replace the .ok()? chain
with a match that logs the Err variant (including the error message) and
continues to return None or propagate an error if you prefer changing the
function to return Result.

---

Nitpick comments:
In `@gradients/amethyst.yaml`:
- Around line 1-44: The gradients YAML files (gradients/*.yaml) can be malformed
and silently reduce templates at runtime; add a lightweight CI job that
validates all gradients/*.yaml before merge by running a schema/lint step (e.g.,
yamllint or a YAML→JSON Schema/validator like yamale/ajv) against a small schema
that enforces required top-level keys (base_stops, direction, blobs, noise,
text_colors), types (arrays for base_stops/blobs/stops, numbers for
cx/cy/r/offsets, strings for color, numeric opacities between 0–1), and required
blob stop fields (name, cx, cy, r, stops with offset/color). Wire the job into
CI to fail on any schema or type/range errors so malformed YAMLs are caught
before src/templates.rs auto-discovery runs.

In `@gradients/hazel.yaml`:
- Around line 27-40: Rename the generic blob entries color_4 and color_5 to
descriptive names reflecting their colors (e.g., change color_4 -> lime and
color_5 -> beige) in the gradients definition, update their corresponding blocks
(cx, cy, r, stops) to use the new names, and ensure any code or templates that
reference color_4 or color_5 are updated to the new identifiers (search for uses
of color_4 and color_5 and replace them with lime and beige to maintain
consistency with other blobs like gold, brown, olive).

In `@gradients/midnight.yaml`:
- Around line 18-19: The gradient stop with color '#3730a3' at offset 0 is
missing an explicit opacity; update the stop object for offset: 0 (color:
'#3730a3') to include an explicit opacity value (e.g., opacity: 1.0) so it no
longer relies on implicit defaults and matches the explicit opacity style used
for the offset: 100 stop.

In `@gradients/moss.yaml`:
- Around line 18-19: The two lime gradient stops include a trailing stop with
opacity but the leading stop at {offset: 0, color: '#65a30d'} omits opacity;
update that first stop in moss.yaml to include an explicit opacity (e.g.,
opacity: 1.0) so both lime stops declare opacity consistently and avoid relying
on renderer defaults.

In `@gradients/terra.yaml`:
- Around line 27-35: Replace the ordinal blob names color_4 and color_5 with
descriptive, semantic identifiers (e.g., terra-warm-center, terra-warm-fade or
names that describe role/position like warm-center and warm-edge) in the YAML so
intent is clear; update every reference to those keys elsewhere in the repo or
asset pipeline to match the new names and preserve the existing properties (cx,
cy, r, stops) exactly so rendering is unchanged.

In `@web/src/routes/`+layout.server.ts:
- Around line 30-63: The current load() function recomputes repoRoot,
reads/parses templates.yaml and gradients, and rebuilds
composedTemplates/allTemplates on every invocation; move that one-time file I/O
and composition to module scope or implement a module-scoped memo (e.g., a
cachedTemplates or initTemplates() with a boolean/Promise) so the YAML parsing
and gradient directory reads run only once at import time, then have load()
return the cached composedTemplates/allTemplates; reference the existing symbols
load, repoRoot, templates.yaml parsing, gradientDefs, composedTemplates and
allTemplates when relocating the logic and ensure load still returns the same
structure.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97b007e2-a3af-44f9-b325-f389fd4308e8

📥 Commits

Reviewing files that changed from the base of the PR and between 5a2ac03 and f5f54cd.

📒 Files selected for processing (78)
  • gradients/amethyst.yaml
  • gradients/arctic.yaml
  • gradients/aurora.yaml
  • gradients/autumn.yaml
  • gradients/berry.yaml
  • gradients/candy.yaml
  • gradients/cedar.yaml
  • gradients/champagne.yaml
  • gradients/citrus.yaml
  • gradients/cloud.yaml
  • gradients/cobalt.yaml
  • gradients/copper-teal.yaml
  • gradients/copper.yaml
  • gradients/coral-navy.yaml
  • gradients/coral.yaml
  • gradients/cosmos.yaml
  • gradients/dawn.yaml
  • gradients/duotone.yaml
  • gradients/dusk.yaml
  • gradients/eclipse.yaml
  • gradients/ember-ice.yaml
  • gradients/ember.yaml
  • gradients/evergreen.yaml
  • gradients/fig.yaml
  • gradients/forest.yaml
  • gradients/galaxy.yaml
  • gradients/garnet.yaml
  • gradients/ginger.yaml
  • gradients/hazel.yaml
  • gradients/heather.yaml
  • gradients/horizon.yaml
  • gradients/indigo-blush.yaml
  • gradients/jade.yaml
  • gradients/lagoon.yaml
  • gradients/lavender.yaml
  • gradients/lilac.yaml
  • gradients/lotus.yaml
  • gradients/maple.yaml
  • gradients/mauve.yaml
  • gradients/midnight.yaml
  • gradients/mint.yaml
  • gradients/moonstone.yaml
  • gradients/moss.yaml
  • gradients/neon.yaml
  • gradients/noir.yaml
  • gradients/noire.yaml
  • gradients/ocean.yaml
  • gradients/opal.yaml
  • gradients/orchid.yaml
  • gradients/peach-lavender.yaml
  • gradients/peach.yaml
  • gradients/pine.yaml
  • gradients/plum-gold.yaml
  • gradients/quartz.yaml
  • gradients/raven.yaml
  • gradients/ruby.yaml
  • gradients/saffron.yaml
  • gradients/sage-terracotta.yaml
  • gradients/sahara.yaml
  • gradients/sapphire.yaml
  • gradients/seaglass.yaml
  • gradients/shadow.yaml
  • gradients/sherbet.yaml
  • gradients/steel.yaml
  • gradients/storm.yaml
  • gradients/sunset.yaml
  • gradients/terra.yaml
  • gradients/thunderstorm.yaml
  • gradients/tropics.yaml
  • gradients/velvet.yaml
  • gradients/voltage.yaml
  • gradients/wine.yaml
  • src/templates.rs
  • templates.yaml
  • web/src/lib/components/playground/TemplateSelector.svelte
  • web/src/lib/stores/playground.svelte.ts
  • web/src/routes/+layout.server.ts
  • web/src/routes/playground/+page.svelte
✅ Files skipped from review due to trivial changes (30)
  • gradients/lilac.yaml
  • gradients/ocean.yaml
  • gradients/noire.yaml
  • gradients/steel.yaml
  • gradients/opal.yaml
  • gradients/dusk.yaml
  • gradients/mauve.yaml
  • gradients/forest.yaml
  • gradients/coral-navy.yaml
  • gradients/fig.yaml
  • gradients/cedar.yaml
  • gradients/duotone.yaml
  • gradients/cosmos.yaml
  • gradients/quartz.yaml
  • gradients/citrus.yaml
  • gradients/neon.yaml
  • gradients/arctic.yaml
  • gradients/horizon.yaml
  • gradients/ember.yaml
  • gradients/aurora.yaml
  • gradients/wine.yaml
  • gradients/jade.yaml
  • gradients/mint.yaml
  • gradients/galaxy.yaml
  • gradients/lotus.yaml
  • gradients/storm.yaml
  • gradients/peach-lavender.yaml
  • gradients/ginger.yaml
  • gradients/sapphire.yaml
  • gradients/heather.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/src/lib/stores/playground.svelte.ts
  • web/src/routes/playground/+page.svelte

cy: 55
r: 55
stops:
- {offset: 0, color: '#d97706'}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Verify how missing opacity is handled in gradient parsing.
rg -n -C4 'parse_gradient_def|opacity|stops' src/templates.rs

Repository: twangodev/ogis

Length of output: 10132


🏁 Script executed:

fd -type f -name "copper.yaml" -o -name "berry.yaml" | head -20

Repository: twangodev/ogis

Length of output: 228


🏁 Script executed:

find . -type f -name "*.yaml" | grep -E "(copper|berry)" | head -20

Repository: twangodev/ogis

Length of output: 133


🏁 Script executed:

echo "=== copper.yaml ===" && sed -n '10,25p' ./gradients/copper.yaml && echo "" && echo "=== berry.yaml ===" && sed -n '10,25p' ./gradients/berry.yaml

Repository: twangodev/ogis

Length of output: 773


Standardize opacity specification for gradient stops across copper.yaml and berry.yaml.

The orange stop at line 18 of copper.yaml (and crimson stop in berry.yaml) omits opacity while other similar stops specify it explicitly. However, this does not cause rendering instability—SVG defaults missing stop-opacity to 1.0 per specification.

The real concern is consistency. Other offset-0 stops in the same gradients (e.g., yellow in copper.yaml) explicitly declare opacity, making the omission inconsistent. Consider either:

  1. Add explicit opacity: 1.0 (or appropriate value matching the design intent) to offset-0 stops that lack it, or
  2. Consistently omit opacity from all stops where the default suffices.

Without additional design context, the proposed value of 0.7 cannot be verified.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/copper.yaml` at line 18, The orange stop in copper.yaml (offset: 0,
color: '#d97706') and the crimson stop in berry.yaml omit an explicit opacity
while other offset-0 stops include it; to standardize, add an explicit opacity:
1.0 to those offset: 0 stops (e.g., the orange stop in copper.yaml and the
crimson stop in berry.yaml) so all offset-0 stops explicitly declare opacity
instead of relying on the SVG default.

Comment on lines +13 to +19
- name: light_violet
cx: 80
cy: 30
r: 45
stops:
- {offset: 0, color: '#c4b5fd'}
- {offset: 100, color: '#c4b5fd', opacity: 0.0}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Make the light_violet stop opacity explicit.

Line 18 omits opacity while the other leading stops in this preset set it explicitly. Keeping this one implicit makes the rendered intensity depend on defaults rather than the preset itself.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/lavender.yaml` around lines 13 - 19, The preset "light_violet"
defines gradient stops where the final stop (stops entry with offset: 100) omits
an explicit opacity; update that stop to include opacity: 0.0 so all leading
stops explicitly declare opacity, ensuring consistent rendering irrespective of
defaults (look for the "name: light_violet" block and its "stops" entries,
especially the stop with offset 100).

Comment on lines +13 to +19
- name: light_peach
cx: 70
cy: 30
r: 50
stops:
- {offset: 0, color: '#fed7aa'}
- {offset: 100, color: '#fed7aa', opacity: 0.0}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Make the light_peach stop opacity explicit.

Line 18 is the only leading stop in this preset without opacity. Even if the intended value is 1.0, it should be written explicitly so this preset does not depend on parser defaults.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gradients/peach.yaml` around lines 13 - 19, The preset "light_peach" has a
leading gradient stop without an explicit opacity (under the stops list for
name: light_peach); update the first stop (offset: 0, color: '#fed7aa') to
include an explicit opacity value (e.g., opacity: 1.0) so the gradient does not
rely on parser defaults.

Comment on lines +723 to +735
// Auto-generate all layout × gradient combinations
let mut gradient_names: Vec<&String> = gradients.keys().collect();
gradient_names.sort();
let mut layout_names: Vec<&String> = layouts.keys().collect();
layout_names.sort();
for gradient_name in &gradient_names {
for layout_name in &layout_names {
let template_name = format!("gradient-{gradient_name}-{layout_name}");
let layout_svg = &layouts[layout_name.as_str()];
let gradient_def = &gradients[gradient_name.as_str()];
let entry =
build_composed_template(&template_name, layout_svg, gradient_name, gradient_def);
register_entry(
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1) Find hardcoded gradient template IDs that do not include a layout suffix.
# Expected: no matches (or an intentional migration list).
rg -nP --type=ts --type=svelte -C2 "\bgradient-[a-z0-9]+(?=['\"])"

# 2) Confirm current composed-name generation points in backend/frontend loaders.
rg -nP -C2 "gradient-\{gradient_name\}-\{layout_name\}|gradient-\$\{gradient\}-\$\{layout\}" src/templates.rs web/src/routes/+layout.server.ts

Repository: twangodev/ogis

Length of output: 507


🏁 Script executed:

#!/bin/bash

# Find CardStack.svelte and inspect it
find . -name "CardStack.svelte" -type f 2>/dev/null | head -5

# Search for all gradient template references in the codebase
# (not limited by file type to catch svelte files)
rg "gradient-[a-z0-9]+" --type-list | grep -i svelte
rg "gradient-[a-z0-9]+" -l 2>/dev/null | head -20

Repository: twangodev/ogis

Length of output: 727


🏁 Script executed:

#!/bin/bash

# Look for the CardStack file with a broader search
rg "CardStack" -l --type ts --type js 2>/dev/null | head -10

# Also search without type restrictions
rg -l "CardStack" . 2>/dev/null | head -10

Repository: twangodev/ogis

Length of output: 110


🏁 Script executed:

#!/bin/bash

# Search for references to 'gradient-' in the codebase (any file type)
# Look for common template references
rg "gradient-galaxy|gradient-[a-z0-9]+" --max-count=50 2>/dev/null

Repository: twangodev/ogis

Length of output: 2739


🏁 Script executed:

#!/bin/bash

# Read the templates.rs file to see the full context of the change
# and check if old gradient templates are still registered
wc -l src/templates.rs

# Get a broader view around line 730 to see if old templates are registered
sed -n '700,760p' src/templates.rs

Repository: twangodev/ogis

Length of output: 2173


🏁 Script executed:

#!/bin/bash

# Search for where gradient templates might be registered before the new code
# Check for any registration of single-name gradient templates (without layout)
rg "register.*gradient|gradient.*register" -B 3 -A 3 src/templates.rs

# Check if there's a fallback or compatibility mechanism
rg "fallback|compat|migrate|legacy" src/templates.rs

Repository: twangodev/ogis

Length of output: 149


🏁 Script executed:

#!/bin/bash

# Check if templates.yaml exists and look for gradient template definitions
find . -name "templates.yaml" -type f 2>/dev/null

# Look for content that might define the old gradient templates
rg "gradient-galaxy|gradient-storm" . 2>/dev/null | grep -v ".git" | head -20

Repository: twangodev/ogis

Length of output: 885


🏁 Script executed:

#!/bin/bash

# Read templates.yaml to see what templates are defined
if [ -f templates.yaml ]; then
  wc -l templates.yaml
  echo "=== First 100 lines ==="
  head -100 templates.yaml
fi

Repository: twangodev/ogis

Length of output: 2653


🏁 Script executed:

#!/bin/bash

# Check git status to understand what changed in this PR
# Look for what templates existed before
git log --oneline -20 2>/dev/null

# Check if there's a git diff for templates.rs
git diff HEAD~1 src/templates.rs 2>/dev/null | head -100

Repository: twangodev/ogis

Length of output: 136


🏁 Script executed:

#!/bin/bash

# Find the load_gradients function
rg -A 30 "fn load_gradients" src/templates.rs

Repository: twangodev/ogis

Length of output: 1240


🏁 Script executed:

#!/bin/bash

# Check if there's any template resolution/lookup logic that might provide fallbacks
rg -B 3 -A 10 "template.*lookup|get.*template|resolve.*template" src/templates.rs | head -50

Repository: twangodev/ogis

Length of output: 40


🏁 Script executed:

#!/bin/bash

# Search for template lookup/retrieval logic in the codebase
rg "get.*template|retrieve.*template|template.*get|TemplateMap" src/templates.rs -B 2 -A 5 | head -80

Repository: twangodev/ogis

Length of output: 1254


🏁 Script executed:

#!/bin/bash

# Check what files are in the gradients directory to see available gradients
ls -la gradients/ 2>/dev/null | head -20

# If gradients is a dir, list the yaml files
find ./gradients -name "*.yaml" -o -name "*.yml" 2>/dev/null | head -20

Repository: twangodev/ogis

Length of output: 1720


🏁 Script executed:

#!/bin/bash

# Look for the old template registration code before the composite template generation
# Check git history or comments that might explain the previous behavior
rg "gradient-" src/templates.rs -B 3 -A 3 | head -100

Repository: twangodev/ogis

Length of output: 403


🏁 Script executed:

#!/bin/bash

# Get all gradient file names
find ./gradients -type f \( -name "*.yaml" -o -name "*.yml" \) -exec basename {} .yaml \; -o -exec basename {} .yml \; | sort

# Also check for the specific gradient names mentioned in the svelte files
echo "=== Checking for specific gradients mentioned in code ==="
for gradient in galaxy storm peach ember jade sapphire midnight ruby lavender cobalt; do
  if [ -f "gradients/${gradient}.yaml" ] || [ -f "gradients/${gradient}.yml" ]; then
    echo "✓ Found: $gradient"
  else
    echo "✗ Missing: $gradient"
  fi
done

Repository: twangodev/ogis

Length of output: 816


🏁 Script executed:

#!/bin/bash

# Get the complete list of all gradient files in a cleaner way
ls gradients/*.yaml gradients/*.yml 2>/dev/null | sed 's|.*/||' | sed 's/\.\(yaml\|yml\)$//' | sort

Repository: twangodev/ogis

Length of output: 589


🏁 Script executed:

#!/bin/bash

# Search for how template requests are handled/resolved
rg "fn.*template|template.*fn" src/templates.rs -A 10 | head -100

# Also check if there's any handling in the web backend for template resolution
rg "template" src/lib.rs -B 2 -A 5 2>/dev/null | head -80

Repository: twangodev/ogis

Length of output: 2700


🏁 Script executed:

#!/bin/bash

# Check the server-side template resolution in the web routes
find ./web -name "*.ts" -o -name "*.js" | xargs rg "template" 2>/dev/null | grep -i "gradient\|resolve\|get\|fetch" | head -30

Repository: twangodev/ogis

Length of output: 512


🏁 Script executed:

#!/bin/bash

# Check the routes file mentioned in the original review
if [ -f "web/src/routes/+layout.server.ts" ]; then
  cat -n web/src/routes/+layout.server.ts
fi

Repository: twangodev/ogis

Length of output: 3560


Update template references in CardStack.svelte and I18nShowcase.svelte to use new composite naming scheme.

The new code generates templates named gradient-{gradient}-{layout} (e.g., gradient-galaxy-centered), but CardStack.svelte and I18nShowcase.svelte still reference old single-name templates like gradient-galaxy and gradient-storm. These will result in template not found errors.

Available layouts from templates.yaml: left-heavy, centered, split, minimal, right-heavy, stacked, banner, editorial, spotlight, reverse-split, top-bar, bottom-bar.

Update at least 18 template references across both files to include a layout suffix, or implement a fallback mechanism to select a default layout when only a gradient name is provided.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/templates.rs` around lines 723 - 735, The template generator now
registers template names as "gradient-{gradient}-{layout}" (see template_name
variable and build_composed_template/register_entry), but CardStack.svelte and
I18nShowcase.svelte still reference old "gradient-{gradient}" names causing
not-found errors; update those files to either (A) change each template
reference to include a layout suffix (use one of the available layouts:
left-heavy, centered, split, minimal, right-heavy, stacked, banner, editorial,
spotlight, reverse-split, top-bar, bottom-bar) for at least 18 occurrences, e.g.
"gradient-galaxy-centered", or (B) implement a fallback lookup function in those
components that, given a gradient name, first attempts to use
"gradient-{gradient}-{layout}" (try a preferred layout like "centered" or
iterate layouts) and only then falls back to older "gradient-{gradient}" if
needed; update references to use that lookup helper so all template resolutions
match the names produced by build_composed_template/register_entry.

Comment on lines +33 to +47
// Read templates.yaml from repo root
const yamlPath = resolve(repoRoot, 'templates.yaml');
const yamlContent = readFileSync(yamlPath, 'utf-8');
const data = parse(yamlContent) as TemplatesYaml;

// Split into base and gradient templates
const base = data.templates.filter((t) => !t.name.startsWith('gradient-'));
const gradients = data.templates.filter((t) => t.name.startsWith('gradient-'));
// Read gradient definitions from gradients/ directory
const gradientsDir = resolve(repoRoot, 'gradients');
const gradientDefs: Record<string, GradientDef> = {};
for (const file of readdirSync(gradientsDir)) {
if (file.endsWith('.yaml') || file.endsWith('.yml')) {
const name = file.replace(/\.ya?ml$/, '');
const content = readFileSync(resolve(gradientsDir, file), 'utf-8');
gradientDefs[name] = parse(content) as GradientDef;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard filesystem/YAML loading to avoid hard failures in root layout load.

At Line 35 and Line 41-46, any missing file or malformed YAML throws and can 500 the entire layout load. Add guarded loading with fallback behavior and explicit logging.

Proposed hardening diff
 export async function load() {
 	const repoRoot = resolve(process.cwd(), '..');

-	// Read templates.yaml from repo root
-	const yamlPath = resolve(repoRoot, 'templates.yaml');
-	const yamlContent = readFileSync(yamlPath, 'utf-8');
-	const data = parse(yamlContent) as TemplatesYaml;
+	// Read templates.yaml from repo root
+	const yamlPath = resolve(repoRoot, 'templates.yaml');
+	let data: TemplatesYaml = { default: '', templates: [] };
+	try {
+		const yamlContent = readFileSync(yamlPath, 'utf-8');
+		data = parse(yamlContent) as TemplatesYaml;
+	} catch (err) {
+		console.error('Failed to load templates.yaml', err);
+	}

 	// Read gradient definitions from gradients/ directory
 	const gradientsDir = resolve(repoRoot, 'gradients');
 	const gradientDefs: Record<string, GradientDef> = {};
-	for (const file of readdirSync(gradientsDir)) {
-		if (file.endsWith('.yaml') || file.endsWith('.yml')) {
-			const name = file.replace(/\.ya?ml$/, '');
-			const content = readFileSync(resolve(gradientsDir, file), 'utf-8');
-			gradientDefs[name] = parse(content) as GradientDef;
-		}
-	}
+	try {
+		for (const file of readdirSync(gradientsDir)) {
+			if (file.endsWith('.yaml') || file.endsWith('.yml')) {
+				const name = file.replace(/\.ya?ml$/, '');
+				const content = readFileSync(resolve(gradientsDir, file), 'utf-8');
+				gradientDefs[name] = parse(content) as GradientDef;
+			}
+		}
+	} catch (err) {
+		console.error('Failed to load gradients', err);
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Read templates.yaml from repo root
const yamlPath = resolve(repoRoot, 'templates.yaml');
const yamlContent = readFileSync(yamlPath, 'utf-8');
const data = parse(yamlContent) as TemplatesYaml;
// Split into base and gradient templates
const base = data.templates.filter((t) => !t.name.startsWith('gradient-'));
const gradients = data.templates.filter((t) => t.name.startsWith('gradient-'));
// Read gradient definitions from gradients/ directory
const gradientsDir = resolve(repoRoot, 'gradients');
const gradientDefs: Record<string, GradientDef> = {};
for (const file of readdirSync(gradientsDir)) {
if (file.endsWith('.yaml') || file.endsWith('.yml')) {
const name = file.replace(/\.ya?ml$/, '');
const content = readFileSync(resolve(gradientsDir, file), 'utf-8');
gradientDefs[name] = parse(content) as GradientDef;
}
}
// Read templates.yaml from repo root
const yamlPath = resolve(repoRoot, 'templates.yaml');
let data: TemplatesYaml = { default: '', templates: [] };
try {
const yamlContent = readFileSync(yamlPath, 'utf-8');
data = parse(yamlContent) as TemplatesYaml;
} catch (err) {
console.error('Failed to load templates.yaml', err);
}
// Read gradient definitions from gradients/ directory
const gradientsDir = resolve(repoRoot, 'gradients');
const gradientDefs: Record<string, GradientDef> = {};
try {
for (const file of readdirSync(gradientsDir)) {
if (file.endsWith('.yaml') || file.endsWith('.yml')) {
const name = file.replace(/\.ya?ml$/, '');
const content = readFileSync(resolve(gradientsDir, file), 'utf-8');
gradientDefs[name] = parse(content) as GradientDef;
}
}
} catch (err) {
console.error('Failed to load gradients', err);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/routes/`+layout.server.ts around lines 33 - 47, The current layout
load reads and parses templates.yaml and iterates gradientsDir with
readFileSync/readdirSync/parse without guards, so missing files or malformed
YAML will throw and 500 the app; wrap the templates.yaml load (yamlPath,
readFileSync, parse) in a try/catch and on failure log the error and fall back
to a safe default TemplatesYaml, and similarly check that gradientsDir exists
before readdirSync and wrap each gradient file read/parse in a try/catch so
invalid files are skipped (log the filename + error) while leaving gradientDefs
(the gradientDefs object) as an empty or partial map rather than letting
exceptions bubble.

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.

2 participants