Skip to content

[OSIG] Replace Templates With Canvas Renderer#35

Merged
gregagi merged 5 commits into
mainfrom
rasul/canvas-renderer
Jun 19, 2026
Merged

[OSIG] Replace Templates With Canvas Renderer#35
gregagi merged 5 commits into
mainfrom
rasul/canvas-renderer

Conversation

@rasulkireev

Copy link
Copy Markdown
Collaborator

Summary

  • replace the fixed template image contract with a typed canvas scene spec using rect, text, and image layers
  • swap the Pillow renderer to deterministic layer painting with bounded custom dimensions and provider-font validation
  • remove template-only tools, renderers, docs, unreachable page templates, and stale tests
  • rename render observability from style to renderer

Tests

  • uv run ruff check .
  • sh -c 'set -a; . ./.env.example; set +a; export DATABASE_URL=sqlite:///db.sqlite3; uv run pytest'
  • sh -c 'set -a; . ./.env.example; set +a; export DATABASE_URL=sqlite:///db.sqlite3; uv run python manage.py makemigrations --check --dry-run'
  • sh scripts/mcp-dev migrate
  • sh scripts/mcp-dev call get_image_contract --json
  • sh scripts/mcp-dev call render_image_preview --input-json '{"spec":{"width":800,"height":450,"background":"#0f172a","layers":[{"kind":"rect","x":40,"y":40,"width":720,"height":370,"color":"#1d4ed8","radius":24},{"kind":"text","x":80,"y":120,"width":620,"text":"Canvas smoke test","font":"helvetica","font_size":56,"color":"#ffffff"}]},"include_image_base64":false}' --json
  • npm run build

Notes

  • make test is blocked locally because Docker compose expects .env; native sqlite pytest is passing.

@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile review

@greptile-apps

greptile-apps Bot commented Jun 18, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the fixed-template image pipeline with a typed canvas renderer built on composable rect, text, and image layers. It also adds a new core/image_url_safety.py module to enforce HTTPS-only, credential-free, public-IP-only image source URLs, addresses the previously flagged gradient background serialization gap via _fill_dump, and improves multi-line text height estimation in _canvas_warnings.

  • Canvas model: ImageSpec now accepts width/height, background (solid or gradient), and up to 50 typed layers; the old template fields (style, title, subtitle, eyebrow, image_url) are removed entirely.
  • Renderer rewrite: core/image_styles.py is replaced with a deterministic layer painter (_draw_rect, _draw_text, _draw_image) using _composite_clipped for safe out-of-bounds handling.
  • Observability: RenderAttempt.style is renamed to renderer (migration 0012), and the unused Image model is dropped (migration 0013 + tasks cleanup).

Confidence Score: 5/5

Safe to merge; the new canvas renderer is well-validated, SSRF mitigations are in place, the gradient serialization bug from the prior review is fixed, and the migrations are correct.

The previous blocking issues (SSRF, gradient background crash, single-line height estimate) are all addressed in this revision. The two remaining findings are both cosmetic: a misleading return-type annotation with a dead None-guard, and align/valign fields on TextLayer being silently ignored when the corresponding bounding dimension is absent. Neither causes incorrect renders or data loss.

No files require special attention. The _layer_bounds annotation and the silent align/valign behaviour are in agent_images/services.py but are non-blocking.

Important Files Changed

Filename Overview
agent_images/services.py Replaces template-based ImageSpec with a typed canvas model (RectLayer, TextLayer, ImageLayer). Two minor logic issues: _layer_bounds never returns None despite its
core/image_styles.py Replaces the old template renderer with a deterministic canvas painter. The gradient rotation now uses math.hypot instead of the previously flagged 2x max-edge formula.
core/image_url_safety.py New module addressing the previously flagged SSRF gap. Enforces HTTPS-only, blocks credentials, private IP literals, localhost, and resolves DNS to verify only global addresses reach the fetch path.
core/migrations/0012_rename_renderattempt_style_renderer.py Renames RenderAttempt.style to RenderAttempt.renderer via a standard Django RenameField migration.
core/migrations/0013_delete_image.py Removes the now-unused Image model from the database. Migration chain is correct and tasks.py no longer references Image.
core/tasks.py Removes save_generated_image and regenerate_and_update_image which relied on the deleted Image model and generate_image_router.
core/tests/test_generate_image_features.py Comprehensive test rewrite covering canvas layers, gradient backgrounds, SSRF rejection, pixel-area limits, text clamp/wrap warnings, inline base64, and redirect re-validation.
agent_images/mcp.py Minimal update: removes list_image_templates tool and keeps four canvas-oriented tools with unchanged signatures.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant MCP as mcp.py
    participant Services as services.py
    participant Styles as image_styles.py
    participant Safety as image_url_safety.py

    Caller->>MCP: render_image_preview(spec: ImageSpec)
    MCP->>Services: render_image(spec)
    Services->>Services: normalize_image_spec(spec)
    Services->>Services: _dimensions_for_spec(spec)
    Services->>Services: _fill_dump(background)
    Services->>Services: _layer_dump(layers[])
    Services->>Services: _canvas_warnings(spec, w, h)
    Services->>Styles: render_canvas_image(render_params)
    Styles->>Styles: _fill_image(background)
    loop each layer
        alt "kind == rect"
            Styles->>Styles: _draw_shadow / _draw_rect
        else "kind == text"
            Styles->>Styles: _draw_text
        else "kind == image"
            Styles->>Safety: "validate_remote_image_url(url, resolve=True)"
            Styles->>Styles: _load_source_image
            Styles->>Styles: _draw_image
        end
    end
    Styles->>Styles: add_watermark (if no pro sub)
    Styles-->>Services: image buffer
    Services->>Services: _record_render_attempt_safely
    Services-->>MCP: render result dict
    MCP-->>Caller: "{spec, warnings, image_base64, ...}"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant MCP as mcp.py
    participant Services as services.py
    participant Styles as image_styles.py
    participant Safety as image_url_safety.py

    Caller->>MCP: render_image_preview(spec: ImageSpec)
    MCP->>Services: render_image(spec)
    Services->>Services: normalize_image_spec(spec)
    Services->>Services: _dimensions_for_spec(spec)
    Services->>Services: _fill_dump(background)
    Services->>Services: _layer_dump(layers[])
    Services->>Services: _canvas_warnings(spec, w, h)
    Services->>Styles: render_canvas_image(render_params)
    Styles->>Styles: _fill_image(background)
    loop each layer
        alt "kind == rect"
            Styles->>Styles: _draw_shadow / _draw_rect
        else "kind == text"
            Styles->>Styles: _draw_text
        else "kind == image"
            Styles->>Safety: "validate_remote_image_url(url, resolve=True)"
            Styles->>Styles: _load_source_image
            Styles->>Styles: _draw_image
        end
    end
    Styles->>Styles: add_watermark (if no pro sub)
    Styles-->>Services: image buffer
    Services->>Services: _record_render_attempt_safely
    Services-->>MCP: render result dict
    MCP-->>Caller: "{spec, warnings, image_base64, ...}"
Loading

Reviews (8): Last reviewed commit: "fix(agent-images): bound canvas layer re..." | Re-trigger Greptile

Comment thread core/image_styles.py Outdated
Comment thread agent_images/services.py
@greptile-apps

greptile-apps Bot commented Jun 18, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the fixed template contract (style, title, subtitle, image_or_logo) with a typed canvas scene spec (RectLayer, TextLayer, ImageLayer) and swaps the Pillow template renderer for a deterministic layer-painting renderer in core/image_styles.py. Supporting changes rename RenderAttempt.style → renderer (with migration), remove the list_image_templates MCP tool, and update all tests to use canvas spec fixtures.

  • New layer model in agent_images/services.py: discriminated union with full field validation, color parsing, font-provider normalization, and canvas-bounds warnings.
  • New Pillow renderer in core/image_styles.py: draws rect/text/image layers in order with opacity, rounding, remote image fetch, and text wrapping; watermark is applied for non-pro profiles.
  • Docs, templates, and dead-code cleanup: job-board template docs, unreachable page templates, and stale test fixtures removed.

Confidence Score: 4/5

Safe to merge with two non-blocking concerns: an incomplete canvas-overflow warning for multi-line text, and an unvalidated URL field on ImageLayer that could be used to probe internal services from the unauthenticated trial endpoint.

The canvas renderer, discriminated-union layer models, and retry/observability pipeline are all well-structured and covered by tests. The migration is minimal and correct. The two flagged issues are non-blocking: the clipping warning gap won't break rendering, and the SSRF risk is a hardening item that also existed under the old image_url field. No data-loss, crash, or auth paths are affected by this change.

agent_images/services.py — the _layer_bounds height approximation and ImageLayer.url field both warrant a second look before this endpoint is gated behind real auth.

Security Review

  • SSRF via ImageLayer.url (agent_images/services.py, ImageLayer): the new ImageLayer type accepts any URL string with no scheme restriction or private-range blocklist. _load_remote_image passes it directly to requests.get, and the trial MCP endpoint is unauthenticated, so an attacker can probe internal addresses (e.g. cloud IMDS, localhost ports).

Important Files Changed

Filename Overview
agent_images/services.py Core rewrite — replaces the template-based ImageSpec with typed canvas layer models (RectLayer, TextLayer, ImageLayer), adds dimension/bounds validation, and re-implements image_contract(). Two issues: TextLayer height underestimation in _layer_bounds leads to silent canvas-overflow, and ImageLayer.url has no http/https-only enforcement (SSRF).
core/image_styles.py New canvas renderer — adds _draw_rect, _draw_text, _draw_image, and render_canvas_image using Pillow. Opacity, clipping, image resizing (cover/contain/stretch), and text wrapping look correct. Old template generator code removed.
agent_images/mcp.py Removes list_image_templates tool, updates docstrings to canvas terminology, and imports are cleaned up. Logic is unchanged from the pre-PR version.
core/migrations/0012_rename_renderattempt_style_renderer.py Simple RenameField migration; dependencies chain correctly to 0011_renderattempt.
core/tasks.py Single-line change: swaps generate_image_router for render_canvas_image. Pre-existing setattr loop in regenerate_and_update_image will silently skip canvas spec keys that don't map to Image model fields, but this is unchanged behavior.
core/tests/test_generate_image_features.py Replaced template-style test fixtures with canvas spec helpers; covers custom dimensions, Google Font provider, color validation, remote image layers, and JPEG determinism.
core/tests/test_render_reliability.py Tests updated to use canvas spec; retry logic, non-transient error paths, and metrics dashboard tests all preserved and passing.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Agent
    participant MCP as MCP Tool
    participant Services as agent_images/services.py
    participant Styles as core/image_styles.py
    participant Observability as core/render_observability.py

    Agent->>MCP: render_image_preview(spec)
    MCP->>Services: render_image(spec, profile)
    Services->>Services: normalize_image_spec(spec)
    note over Services: validate layers, check canvas bounds, emit warnings
    Services->>Services: track_profile_usage(profile)
    loop Up to OSIG_RENDER_MAX_ATTEMPTS
        Services->>Styles: render_canvas_image(render_params)
        Styles->>Styles: draw rect / text / image layers
        alt render fails (transient)
            Styles-->>Services: Exception
            Services->>Observability: "record_render_attempt(success=False)"
            Services->>Services: retry if transient
        else render succeeds
            Styles-->>Services: BytesIO buffer
            Services->>Observability: "record_render_attempt(success=True)"
            Services-->>MCP: "{spec, image_base64, metadata}"
        end
    end
    MCP-->>Agent: result dict
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Agent
    participant MCP as MCP Tool
    participant Services as agent_images/services.py
    participant Styles as core/image_styles.py
    participant Observability as core/render_observability.py

    Agent->>MCP: render_image_preview(spec)
    MCP->>Services: render_image(spec, profile)
    Services->>Services: normalize_image_spec(spec)
    note over Services: validate layers, check canvas bounds, emit warnings
    Services->>Services: track_profile_usage(profile)
    loop Up to OSIG_RENDER_MAX_ATTEMPTS
        Services->>Styles: render_canvas_image(render_params)
        Styles->>Styles: draw rect / text / image layers
        alt render fails (transient)
            Styles-->>Services: Exception
            Services->>Observability: "record_render_attempt(success=False)"
            Services->>Services: retry if transient
        else render succeeds
            Styles-->>Services: BytesIO buffer
            Services->>Observability: "record_render_attempt(success=True)"
            Services-->>MCP: "{spec, image_base64, metadata}"
        end
    end
    MCP-->>Agent: result dict
Loading

Reviews (2): Last reviewed commit: "feat: replace templates with canvas rend..." | Re-trigger Greptile

Comment thread agent_images/services.py
Comment thread agent_images/services.py
@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile review

@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile review\n\nPlease review the latest head commit 3d285ca after the SSRF and text-warning fixes.

Comment thread agent_images/services.py
@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile review

Comment thread core/image_styles.py
@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile review

@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile review\n\nPlease review the latest head commit 52c43d9. All currently visible Greptile review threads are resolved.

@rasulkireev

Copy link
Copy Markdown
Collaborator Author

@greptile-apps review

@gregagi gregagi merged commit 344059d into main Jun 19, 2026
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