Skip to content

fix: split subfolder from filename in getMediaUrl for correct /api/view thumbnails - #12438

Open
phazei wants to merge 40 commits into
Comfy-Org:mainfrom
phazei:fix/widget-select-subfolder-thumbnail-url
Open

fix: split subfolder from filename in getMediaUrl for correct /api/view thumbnails#12438
phazei wants to merge 40 commits into
Comfy-Org:mainfrom
phazei:fix/widget-select-subfolder-thumbnail-url

Conversation

@phazei

@phazei phazei commented May 23, 2026

Copy link
Copy Markdown

Summary

Fix broken thumbnails in the image combo dropdown for values containing subfolder paths (e.g. subfolder/image.png).

Changes

getMediaUrl in useWidgetSelectItems.ts now uses the existing parseImageWidgetValue utility to split subfolder-prefixed filenames into separate filename and subfolder query parameters when constructing /api/view URLs. Added a test case for nested subfolder paths.

Review Focus

The /api/view endpoint calls os.path.basename(filename) on the filename param, stripping any embedded path components. It already supports a separate subfolder query parameter (and parseImageWidgetValue already exists for exactly this split), but getMediaUrl wasn't using it. Flat filenames are unaffected — parseImageWidgetValue returns an empty subfolder string and no param is added.

Fixes #12437

E2E test rationale: The fix modifies pure URL construction logic in getMediaUrl, which is fully validated by the added unit test. An E2E regression test would require a backend serving images from nested subfolders to verify thumbnail rendering, making it fragile and environment-dependent. The unit test directly asserts the correct filename and subfolder query parameter splitting, which is the root cause of the bug.

@phazei
phazei requested a review from a team May 23, 2026 07:28
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label May 23, 2026
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

getMediaUrl() now separates nested image paths into filename and subfolder query parameters. Distribution builds scan for Datadog RUM references. Biweekly releases accept an optional target branch override and pass it to the resolver.

Changes

Nested Image Preview URLs

Layer / File(s) Summary
Preview URL parsing and construction
src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts
Uses parseImageWidgetValue() to set filename, resolve type, and include an optional subfolder in /api/view requests.
Nested path preview validation
src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts
Verifies basename, URL-encoded subfolder, and type=input query parameters.

Distribution and Release Workflow Controls

Layer / File(s) Summary
Distribution telemetry scanning
.github/workflows/ci-dist-telemetry-scan.yaml
Scans built HTML and JavaScript assets for Datadog RUM references and fails the job when matches are found.
Release branch override
.github/workflows/release-biweekly-comfyui.yaml
Adds the optional target_branch input, reports its value, and passes it to the release resolver through TARGET_BRANCH.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: core/1.48

Suggested reviewers: christian-byrne, benceruleanlu


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The pull request also changes unrelated telemetry scanning and release workflow behavior. Move the telemetry scan and release workflow changes to separate pull requests, or link issues that define those requirements.
End-To-End Regression Coverage For Fixes ❓ Inconclusive The PR metadata does not provide the PR title or commit subjects, so the required bug-fix signal cannot be verified. Provide the PR title and commit subjects to determine whether the check applies.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation separates filename and subfolder parameters and adds coverage for nested paths, satisfying issue #12437.
Adr Compliance For Entity/Litegraph Changes ✅ Passed The PR changes URL construction, a unit test, and workflow inputs; the diff adds no entity mutations, god-object APIs, ECS methods, or extension-facing callback changes.
Title check ✅ Passed The title clearly summarizes the main fix: splitting subfolders from filenames for correct /api/view thumbnail URLs.
Description check ✅ Passed The description explains the issue, implementation, test coverage, review focus, and linked issue; screenshots are not applicable.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

🎭 Playwright: ⏳ Running...

@phazei
phazei force-pushed the fix/widget-select-subfolder-thumbnail-url branch 2 times, most recently from 4424f86 to a15b9b6 Compare May 23, 2026 21:59
@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

🎨 Storybook: loading Building...

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 23, 2026
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

✅ All contributors have signed the CLA. Thank you! This PR is ready to be merged.
Posted by the CLA Assistant Lite bot.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

🎨 Storybook: 🚧 Building...

🎭 Playwright: ⏳ Running...

@phazei

phazei commented Jul 4, 2026

Copy link
Copy Markdown
Author

I have read and agree to the Contributor License Agreement

comfy-legal added a commit to Comfy-Org/comfy-cla that referenced this pull request Jul 4, 2026
comfy-pr-bot and others added 15 commits July 9, 2026 15:46
…Comfy-Org#13553)

Backport of Comfy-Org#13536 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: AustinMroz <austin@comfy.org>
…RE-329) (Comfy-Org#13580)

Backport of Comfy-Org#13330 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Terry Jia <terryjia88@gmail.com>
…to Create Bounding Boxes (Comfy-Org#13582)

Backport of Comfy-Org#13376 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Terry Jia <terryjia88@gmail.com>
…ogs shift focus + BYOK secrets E2E (Comfy-Org#13621)

Backport of Comfy-Org#13510 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Matt Miller <matt@miller-media.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…r-driven provider options (Comfy-Org#13623)

Backport of Comfy-Org#13546 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Matt Miller <matt@miller-media.com>
… the subgraph host (Comfy-Org#13637)

Backport of Comfy-Org#13542 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
…erated node defs + preserve raw messages on compile errors (Comfy-Org#13651)

Backport of Comfy-Org#13631 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Connor Byrne <c.byrne@comfy.org>
Co-authored-by: xmarre <54859656+xmarre@users.noreply.github.com>
Patch version increment to 1.47.8

**Base branch:** `core/1.47`

Co-authored-by: comfy-pr-bot <172744619+comfy-pr-bot@users.noreply.github.com>
…ide + major versions in resolve-comfyui-release (Comfy-Org#13661)

Backport of Comfy-Org#13660 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Org#13665)

Future generations should be taken care of with the new processing.
Patch version increment to 1.47.9

**Base branch:** `core/1.47`

Co-authored-by: comfy-pr-bot <172744619+comfy-pr-bot@users.noreply.github.com>
…e's subfolder (Comfy-Org#13759)

Backport of Comfy-Org#13744 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Angelo Lesniak <angelo.lesniak@orf.at>
…widget value store (Comfy-Org#13787)

Backport of Comfy-Org#13773 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Austin Mroz <austin@comfy.org>
… definitions (Comfy-Org#13790)

Backport of Comfy-Org#13789 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
…error tab copy (Comfy-Org#13793)

Backport of Comfy-Org#13684 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
jaeone94 and others added 20 commits July 22, 2026 14:15
…mfy-Org#13933)

Backport of Comfy-Org#13869 to `core/1.47`.

The automatic backport conflicted in `useAuthActions.test.ts` because
this release branch does not contain the newer telemetry mock. The
resolution preserves the branch-specific test setup and applies only the
distribution mock needed by the fix.

## Validation

- `pnpm test:unit src/composables/auth/useAuthActions.test.ts` (13
tests)
- `pnpm typecheck`
- Changed-file formatting, oxlint, and ESLint checks
- `pnpm knip` (push hook)
…d input lookup (Comfy-Org#13979)

Backport of Comfy-Org#13865 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lue in node tooltip (Comfy-Org#13977)

Backport of Comfy-Org#13864 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…up→subgraph connection + nested-promotion value) (Comfy-Org#13974)

Backport of Comfy-Org#13860 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
Comfy-Org#13695)

Backport of Comfy-Org#13608 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
…omoted-input reorder jump (Comfy-Org#14004)

Backport of Comfy-Org#13866 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
…hader from widget store (Comfy-Org#14008)

Backport of Comfy-Org#13874 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ac in Drag Navigation mode (Comfy-Org#14012)

Backport of Comfy-Org#13989 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ow size at runtime (size Proxy) (Comfy-Org#14016)

Backport of Comfy-Org#13867 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s in live preview and histogram (Comfy-Org#14019)

Manual backport of Comfy-Org#13875 to `core/1.47` (auto-backport bot failed on
conflict in `useProcessedWidgets.ts` imports).

Under ADR 0009 link-only promotion, a promoted proxy widget's live value
lives at the host widget key while its node output (histogram) is keyed
by the interior source node's locator. GLSL live preview and curve/range
histogram read the wrong key, so editing a promoted curve/range proxy
never re-rendered. Fix resolves promoted sources to their host widget
key / source node locator.

Cherry-pick of merge commit d6bc36c. Conflict resolved: kept the new
`graphTraversalUtil` import block (`executionIdToNodeLocatorId`,
`getExecutionIdFromNodeData`); dropped the `hasErrorForSlot` import that
is unused on this branch.

Gates: typecheck OK, unit 88/88, eslint/oxlint/oxfmt OK.

Backport of Comfy-Org#13875. Do not merge without release owner review.

Co-authored-by: Terry Jia <terryjia88@gmail.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
… size-Proxy (trim comments, cut spec-mechanics tests, hoist mutations) (Comfy-Org#14027)

Backport of Comfy-Org#14025 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Patch version increment to 1.47.10

**Base branch:** `core/1.47`

Co-authored-by: comfy-pr-bot <172744619+comfy-pr-bot@users.noreply.github.com>
…ck (Comfy-Org#14065)

Backport of Comfy-Org#14054 to `core/1.47`

Manually resolved after the automatic backport conflicted with
branch-specific component changes.

Co-authored-by: Jaewon Yoon <jaewon@comfy.org>
Co-authored-by: Amp <amp@ampcode.com>
…h widgets (Comfy-Org#14116)

Backport of Comfy-Org#14113 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: AustinMroz <austin@comfy.org>
Comfy-Org#14177) (Comfy-Org#14188)

Manual backport of Comfy-Org#14177 to `core/1.47` for the next 1.47 patch
release.

Cherry-picked from upstream merge commit `c9199e9`.

## Why

Keeps the Run button warning icon synchronized with missing nodes,
models, and media in the active workflow instead of retaining stale
state after switching workflows.

## Conflict resolution

- **Eight Playwright PNG snapshots** -- accepted the snapshots updated
by the upstream PR.
- **`LinearControls.vue`** -- preserved the 1.47 `isActiveSubscription`
billing contract and release-branch layout, then applied the
missing-resource icon state to both Run buttons.
- **`LinearControls.test.ts`** -- preserved the 1.47 subscription test
contract while adding the upstream missing-resource cases.
- **`executionErrorStore.test.ts`** -- kept the new `hasMissingError`
coverage and seeded validation errors through the 1.47 writable store
state because `recordNodeErrors` is not available on this branch.
- **`ComfyQueueButton.test.ts`** -- seeded prompt errors through the
1.47 writable store state because `recordPromptError` is not available
on this branch.

## Validation

- `pnpm typecheck`
- Targeted unit tests: 51 passed
- ESLint on changed TypeScript and Vue files
- oxfmt check on changed text files

Local Playwright execution was skipped; the upstream PR includes its
targeted workflow-switch scenario and updated snapshots, and this PR
will run the release-branch CI suite.

Original PR: Comfy-Org#14177
Original commit: `c9199e9464d1b0f74f8196159483395033a0d834`

---------

Co-authored-by: github-actions <github-actions@github.com>
…aving in the mask editor (Comfy-Org#14191)

Backport of Comfy-Org#13758 to `core/1.47`

Automatically created by backport workflow.

Co-authored-by: Angelo Lesniak <angelo.lesniak@orf.at>
Co-authored-by: Terry Jia <terryjia88@gmail.com>
## Summary

Backport the ordered frontend observability stack to `core/1.47` for the
next 1.47 hotfix.

## Changes

Applied in dependency order:

1. Comfy-Org#13691 — initialize Cloud RUM from frontend bootstrap
2. Comfy-Org#13708 — initialize the Datadog RUM telemetry provider
3. Comfy-Org#13767 — record workflow execution vitals
4. Comfy-Org#13764 — attribute workflow vitals to origin views
5. Comfy-Org#14060 — tag RUM traffic with bucket and version
6. Comfy-Org#14094 — inline Cloud Sentry telemetry
7. Comfy-Org#14083 — track manual refresh actions

## Review Focus

All six commits added after Comfy-Org#13691 cherry-picked cleanly with no manual
conflict resolution:

- `80ebc1832b376d9f2a00f672c698e010195977e3`
- `131881284842d436e7f2794fd2b2c541032a3430`
- `b5d17f9106390bfd5e1259d3a20249e611d93817`
- `928435602c4e5a052eb9a7d57e872455f947d0d1`
- `1039660f9b279efb767147e2e31e4d3e4c9f0788`
- `112952b226a5c68df7a90ed462c104dee8ab490f`

For Comfy-Org#13691, the `pnpm-lock.yaml` conflict was resolved by retaining the
target branch's existing dependency versions and regenerating after
adding Datadog. Its lockfile diff contains only the Datadog dependency
graph.

## Validation

- 7 focused Vitest files: 132 tests passed
- Full unit suite: 969 files passed, 12,722 tests passed, 8 skipped
- `pnpm typecheck`
- `pnpm build`
- `pnpm lint`
- `pnpm format:check`
- `pnpm knip`
- `git diff --check`

---------

Co-authored-by: huang47 <157390+huang47@users.noreply.github.com>
…Org#14231)

## Summary

Backport Comfy-Org#14084 to `core/1.47`, resolving the release-branch conflicts
without importing unrelated newer-`main` billing or governance behavior.

## Changes

- **What**: Adds bounded workflow context, queue-trigger attribution,
submission/queue/execution timing, and one terminal `workflow_execution`
vital per attempt.
- **Dependencies**: Builds on the frontend observability stack already
backported in Comfy-Org#14210.

## Review Focus

The conflict resolution keeps the 1.47 execution-error store API while
applying Comfy-Org#14084's structured workflow status and timing metadata.

Validation:
- 4 focused Vitest files, 136 tests passed
- `pnpm typecheck`
- Commit hooks: oxfmt, Oxlint, ESLint
- Pre-push `pnpm knip`
- `git diff --check`

Created by Codex

Co-authored-by: huang47 <157390+huang47@users.noreply.github.com>
Patch version increment to 1.47.11

**Base branch:** `core/1.47`

Co-authored-by: comfy-pr-bot <172744619+comfy-pr-bot@users.noreply.github.com>
@phazei
phazei force-pushed the fix/widget-select-subfolder-thumbnail-url branch from b802bee to b578378 Compare August 3, 2026 08:24
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Aug 3, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci-dist-telemetry-scan.yaml:
- Around line 117-129: Update the Datadog RUM scan condition in the workflow’s
rg block to distinguish ripgrep’s exit statuses: allow only status 1 (no
matches) to continue successfully, fail the step for status 0 (matches) and
status 2 or other errors, and preserve the existing error and success messages
where applicable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c96b4350-76a8-4bbc-ad78-2959e1c62f75

📥 Commits

Reviewing files that changed from the base of the PR and between a15b9b6 and b578378.

📒 Files selected for processing (2)
  • .github/workflows/ci-dist-telemetry-scan.yaml
  • .github/workflows/release-biweekly-comfyui.yaml

Comment on lines +117 to +129
if rg --no-ignore -n \
-g '*.html' \
-g '*.js' \
-e '@datadog/browser-rum' \
-e '041a9897-5516-4b1f-a245-1a9aa6895488' \
-e 'pub7704486e5b64eb4ff6f62891cda45559' \
-e 'comfy-cloud-frontend' \
dist; then
echo '❌ ERROR: Datadog RUM references found in dist assets!'
echo 'Datadog RUM must be properly tree-shaken from OSS builds.'
exit 1
fi
echo '✅ No Datadog RUM references found'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== workflow excerpt =="
sed -n '90,145p' .github/workflows/ci-dist-telemetry-scan.yaml 2>/dev/null || true

echo
echo "== relevant rg behavior in sandbox =="
tmp=$(mktemp -d)
echo "text" > "$tmp/file.txt"
mkdir -p "$tmp/subdir"
echo "no matches" > "$tmp/subdir/file.no"
echo "exact `@datadog/browser-rum` ref" > "$tmp/file.txt"

printf "missing target with exact pattern: %d\n" "$(rg --no-ignore -n -e '`@datadog/browser-rum`' missing 2>/dev/null >/dev/null || true || true; echo $?)"
printf "missing target with no exact pattern: %d\n" "$(rg --no-ignore -n -e 'PATTERN_DO_NOT_EXIST' missing 2>/dev/null >/dev/null || true || true; echo $?)"
printf "match with exact pattern: %d\n" "$(rg --no-ignore -n -e '`@datadog/browser-rum`' "$tmp/file.txt" >/dev/null || echo $?)"
printf "no match with exact pattern: %d\n" "$(rg --no-ignore -n -e '`@datadog/browser-rum`' "$tmp/file.no" >/dev/null || echo $?)"

echo
echo "== release artifact references to dist/artifacts =="
rg -n "upload-artifact|dist/|dist artifacts|artifacts" .github -S || true

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 7989


Fail the Datadog RUM scan when rg reports an error.

rg returns status 2 for invocation/I/O errors, while status 1 means no matches. This step treats both as successful, so a failed scan can print the success message and allow the workflow to continue. Accept only status 1 as clean.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci-dist-telemetry-scan.yaml around lines 117 - 129, Update
the Datadog RUM scan condition in the workflow’s rg block to distinguish
ripgrep’s exit statuses: allow only status 1 (no matches) to continue
successfully, fail the step for status 0 (matches) and status 2 or other errors,
and preserve the existing error and success messages where applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WidgetSelectDropdown.getMediaUrl() does not split subfolder from filename, breaking thumbnails for subfolder-prefixed combo values

7 participants