Codebase review, CLAUDE.md and a prioritised roadmap - #123
Open
MehranMarxian wants to merge 3 commits into
Open
MehranMarxian wants to merge 3 commits into
MehranMarxian wants to merge 3 commits into
Conversation
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
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Mehran · project thread
What changed and why
Three documentation files, one commit each, no product code.
docs/codebase-review.md(new). Architecture map, how a preset flows frompresetRegistryto 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.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 todocs/ORCHESTRATION.mdfor the invariants and CSS traps rather than copying them.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
cancelPromptfalls back to a globalPOST /interruptwhen our prompt isn't pending (src/comfy/comfyClient.ts:501-526), which interrupts whatever is running on the shared ComfyUI.src/ui/tools/livePainting.ts:601,621,631,650).Originor 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.docs/index.html:10,38). These are the strings search engines and LLMs quote.Medium
src/photoshop/modelFileDestination.ts:43-46). Downloads are checked by size only, not by hash.src/ui/App.ts:3071).ComfyClientrequest has a timeout.App.tshas regrown to 9,976 lines after being cut to 5,842.tests/is not type-checked; including it gives 102 errors.bridge/on its own dependencies, and never runs e2e.object_infodrift 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
.partdownloads with hash checks (R3).What I verified
Run on this branch after the changes:
The review's other numbers come from
npm audit,npm outdated,npm audit --package-lock-onlyinbridge/, atscrun that includestests/, and a grep for unused exports. The last 15 CI runs on GitHub are all green.What I could NOT verify
prompt_idon/interrupt. Recent builds do, as far as I know. R1 depends on this, so I'll check it first.Originheader UXP's WebSocket sends. R2 depends on this.docs/llms.txt. I checked only the wording rule, not feature accuracy.What Mehran should check in Photoshop
No product code changed, so the plugin should behave exactly as v0.37.0. One quick check:
npm run build. In the UXP Developer Tool, click Load ondist/manifest.json.Open questions
127.0.0.1:8188, as you said.docs/ORCHESTRATION.md:75says your own instance is on:8190. Which is current?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 getrequire. CLAUDE.md describes the real mechanism.docs/ORCHESTRATION.mdbe 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