Skip to content

Codebase review, CLAUDE.md and a prioritised roadmap - #123

Open
MehranMarxian wants to merge 3 commits into
mainfrom
docs/codebase-review-vsph9b
Open

MehranMarxian wants to merge 3 commits into
mainfrom
docs/codebase-review-vsph9b

Conversation

@MehranMarxian

Copy link
Copy Markdown
Owner

Requested by Mehran · project thread

What changed and why

Three documentation files, one commit each, no product code.

  1. docs/codebase-review.md (new). Architecture map, how a preset flows from presetRegistry to a ComfyUI graph to a layer, test coverage gaps, tech debt, security, dependency freshness and CI health, with 20 findings ranked high/med/low and file:line references. I chose this location so the review lives in the repo next to the roadmap it feeds, instead of only in this description.
  2. CLAUDE.md (new, 137 lines). Your permanent rules, commands, architecture, how to add a preset, UXP traps, shared ComfyUI rules, wording and licence rules, and release steps. It points to docs/ORCHESTRATION.md for the invariants and CSS traps rather than copying them.
  3. docs/roadmap.md (updated in place). A new "Prioritised plan" section with reliability, ease of use, productivity and staying-up-to-date items, each with reason, size (S/M/L) and verification, plus a five-item "do next" order. Existing sections are kept. The opening sentence no longer says "Photoshop UXP plugin".

Ranked findings (summary)

High

  • Cancel can stop someone else's job. cancelPrompt falls back to a global POST /interrupt when our prompt isn't pending (src/comfy/comfyClient.ts:501-526), which interrupts whatever is running on the shared ComfyUI.
  • Live Painting hits that fallback routinely. It cancels prompts that have already finished (src/ui/tools/livePainting.ts:601,621,631,650).
  • Agent Bridge is open to web pages. Browsers don't apply CORS to WebSockets and the hub checks no Origin or token (bridge/src/hub.mjs:59-76). A page can drive the panel, or pose as a panel and bump the real one off (bridge/src/hubRouter.mjs:310-316), while the hub runs.
  • Wording. The landing page meta description and JSON-LD still say "Photoshop UXP plugin" (docs/index.html:10,38). These are the strings search engines and LLMs quote.

Medium

  • Bridge runtime deps have 1 high and 3 moderate advisories.
  • Model downloads write to the final filename, so an interrupted download looks like a real model to ComfyUI (src/photoshop/modelFileDestination.ts:43-46). Downloads are checked by size only, not by hash.
  • A raw "HTTP 400" reaches artists. Text to Image has no friendly mapping (src/ui/App.ts:3071).
  • No ComfyClient request has a timeout.
  • App.ts has regrown to 9,976 lines after being cut to 5,842.
  • tests/ is not type-checked; including it gives 102 errors.
  • CI never installs or tests bridge/ on its own dependencies, and never runs e2e.
  • There is no Dependabot. Vite is 2 majors behind; TypeScript and Vitest are 1 major behind.
  • Nothing catches ComfyUI object_info drift before a release.

Low: dead code (9 unused exports and 3 orphan files), more wording spots, a 698 KB single bundle, docs drift (CONTRIBUTING says Node 18; ORCHESTRATION.md is dated), one preset labelled stable but described as experimental, and broad manifest permissions (already justified).

Recommended next PRs, in order

  1. Cancel only our own prompt (R1).
  2. Close the bridge to browser origins (R2).
  3. Fix the public wording, plus a test that keeps it fixed (E1).
  4. Explain HTTP 400s in plain words (E2).
  5. Safe .part downloads with hash checks (R3).

What I verified

Run on this branch after the changes:

> npm run typecheck
> tsc --noEmit
(exit 0)

> npm run lint
> eslint .
(exit 0)

> npm test
 Test Files  110 passed (110)
      Tests  1094 passed (1094)
   Duration  23.06s

> npm run build
../dist/assets/index-DolD-nV8.js   697.87 kB │ gzip: 153.18 kB
✓ built in 1.70s
Copied UXP assets to dist.

The review's other numbers come from npm audit, npm outdated, npm audit --package-lock-only in bridge/, a tsc run that includes tests/, and a grep for unused exports. The last 15 CI runs on GitHub are all green.

What I could NOT verify

  • Nothing was run in Photoshop or against a live ComfyUI.
  • Whether your ComfyUI version honours prompt_id on /interrupt. Recent builds do, as far as I know. R1 depends on this, so I'll check it first.
  • What Origin header UXP's WebSocket sends. R2 depends on this.
  • Every claim in README and docs/llms.txt. I checked only the wording rule, not feature accuracy.
  • The dead-code list comes from grep. Each removal still needs a build and a Photoshop load.

What Mehran should check in Photoshop

No product code changed, so the plugin should behave exactly as v0.37.0. One quick check:

  1. In GitHub Desktop, choose Current Branch → docs/codebase-review-vsph9b, then Fetch origin.
  2. Run npm run build. In the UXP Developer Tool, click Load on dist/manifest.json.
  3. Open Plugins → OpenLayer. The footer should read v0.37.0. Run one Text to Image.

Open questions

  • CLAUDE.md says the shared ComfyUI is on 127.0.0.1:8188, as you said. docs/ORCHESTRATION.md:75 says your own instance is on :8190. Which is current?
  • You described setup() as registered from an inline head script. It is actually a separate non-deferred file, src/panelBootstrap.js, loaded in <head>. The file explains that inline blocks don't reliably get require. CLAUDE.md describes the real mechanism.
  • Should docs/ORCHESTRATION.md be refreshed or retired now that CLAUDE.md exists? I left it untouched and linked it, because its invariants and CSS-trap sections are still correct.

🤖 Generated with Claude Code

https://claude.ai/code/session_012jnnd7r9nfoeNcfrgkT5tC


Generated by Claude Code

Maps src/, bridge/, scripts/, tests/ and docs/, traces how a preset
becomes a ComfyUI graph and then a layer, and ranks 20 findings high,
medium or low with file and line references. The high ones: cancel can
fall back to a global ComfyUI interrupt on a shared instance, Live
Painting reaches that fallback routinely, the Agent Bridge hub accepts
connections from any web page, and the landing page meta description
still uses Photoshop as an adjective. No product code changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012jnnd7r9nfoeNcfrgkT5tC
Captures the working rules, commands, architecture, how to add a
preset, the UXP traps, the shared ComfyUI rules, the wording and
licence rules, and the release steps, in under 140 lines. It points to
docs/ORCHESTRATION.md for the invariants and CSS traps rather than
copying them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012jnnd7r9nfoeNcfrgkT5tC
Adds reliability, ease of use, productivity and staying-up-to-date
items, each with its reason, a size and how it would be verified, plus
a five-item do-next order. Also replaces the opening sentence's
"Photoshop UXP plugin" with "UXP plugin for Photoshop".

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012jnnd7r9nfoeNcfrgkT5tC
@MehranMarxian MehranMarxian self-assigned this Sep 30, 2026

This branch has not been deployed

No deployments
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