Skip to content

[Spec 0053] af open Image Support#103

Merged
waleedkadous merged 3 commits into
mainfrom
builder/0053-af-open-image-support
Dec 13, 2025
Merged

[Spec 0053] af open Image Support#103
waleedkadous merged 3 commits into
mainfrom
builder/0053-af-open-image-support

Conversation

@waleedkadous
Copy link
Copy Markdown
Contributor

Summary

Extends af open to display images (PNG, JPG, GIF, WebP, SVG) in the dashboard viewer instead of showing binary garbage or errors.

Changes

  • Add image extension detection in open-server.ts
  • Add MIME type mapping and /api/image endpoint for serving binary images
  • Add image viewer UI with zoom controls (Fit, 100%, +, -)
  • Display image dimensions and file size in header
  • Hide code editing UI for image files

Files Changed

File Description
packages/codev/src/agent-farm/servers/open-server.ts Backend: image detection, MIME types, /api/image endpoint
packages/codev/templates/open.html Frontend: image viewer UI, zoom controls, styling
codev/reviews/0053-af-open-image-support.md Review document

3-Way Review Summary

Model Verdict Key Feedback
Gemini APPROVE Clean implementation, correct decision to disable line-based annotations for images
Claude APPROVE Full spec coverage, good error handling, clean code patterns
Codex REQUEST_CHANGES Annotation system disabled for images

Annotation System Discussion

The spec states "Same annotation system available (for annotating screenshots)." The implementation disables the line-based annotation UI for images because:

  1. Line-based annotations don't map to images: The existing system is tied to source code lines
  2. Image annotation requires a new system: Coordinate-based regions, not line numbers
  3. Gemini and Claude agreed: This is the correct technical decision

Codex disagrees: Argues this is a hard spec requirement that must be met.

Recommendation: If image annotation is needed, create a separate spec for coordinate-based annotation (significant new feature). The current implementation meets all other success criteria.

Test Plan

  • af open screenshot.png displays the image
  • Zoom controls work (fit, 100%, +/-)
  • Image dimensions shown in header
  • SVG renders correctly
  • Non-image files still work as before
  • Server returns 400 for /api/image on non-image files

- Add image extension detection (png, jpg, jpeg, gif, webp, svg)
- Add MIME type mapping for serving images with correct Content-Type
- Add /api/image endpoint for serving raw image data
- Add image viewer UI with zoom controls (Fit, 100%, +, -)
- Display image dimensions and file size in header
- Hide code editing UI for image files
@waleedkadous
Copy link
Copy Markdown
Contributor Author

Architect Integration Review (3-Way)

Verdict: APPROVED (after spec clarification)

Review Summary

Model Verdict
Gemini APPROVE
Codex REQUEST_CHANGES
Claude REQUEST_CHANGES

Both REQUEST_CHANGES cited missing annotation system per spec. Spec has been updated (commit 9aa90aa) to clarify that annotations are intentionally not supported for images - line-based system doesn't apply.

Action Required

Please merge this PR. The implementation is complete and correct per the updated spec.


🏗️ Architect integration review

@waleedkadous waleedkadous merged commit 7c04fa8 into main Dec 13, 2025
2 checks passed
@waleedkadous waleedkadous deleted the builder/0053-af-open-image-support branch January 8, 2026 19:05
waleedkadous added a commit that referenced this pull request May 5, 2026
Codex review-phase REQUEST_CHANGES (HIGH): the spec's success criterion #103
('codev init produces arch.md/lessons-learned.md') and the new arch.md
template's claim ('propagates via codev init/adopt') both overstate what the
framework actually does. Verified in init.ts:92-93 and adopt.ts:128: both
files carry the explicit comment 'Framework files (protocols, roles,
consult-types, templates) are NOT copied. They resolve at runtime from the
installed npm package via the unified file resolver.' This is a deliberate
foundational design decision that affects every framework template, not just
arch.md/lessons-learned.md.

Aligning the artifacts with reality (option chosen) rather than flipping the
design decision (rejected as out of scope):
- codev/templates/arch.md: 'Note on propagation' section rewritten to state
  honestly that init/adopt/update do NOT copy framework templates; provides
  a manual-copy command (require.resolve) for projects opting in.
- codev-skeleton/templates/arch.md: byte-identical mirror.
- codev/reviews/723-*.md: smoke-test section updated with 'Spec deviation
  noted' explanation; success-criteria final check updated to reference it.

Gemini APPROVE and Claude APPROVE stand. Claude's two minor observations
(porch artifact duplication; stale Vitest flag in approved spec) are
acknowledged but not actioned as documented in the rebuttal.
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.

1 participant