Handle HTML fallbacks for node help and skip blueprint docs #7021
Conversation
📝 WalkthroughWalkthroughAdds Blueprint node source handling to the help system with early return logic. Refactors error handling in NodeHelpService by introducing a new private tryFetchMarkdown helper method to centralize safe Markdown content fetching across fetchNodeHelp, fetchCustomNodeHelp, and fetchCoreNodeHelp. Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/28/2025, 01:55:38 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/28/2025, 02:05:19 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🔴 +963 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 949 kB (baseline 949 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 139 kB (baseline 139 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.84 MB (baseline 3.84 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
|
@codex review |
dosubot
bot
added
the
size:M
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/services/nodeHelpService.ts (3)
26-44: Custom node help fallback flow looks solid; consider richer error contextThe locale-first then non-locale fallback with
lastErrorgives predictable behavior for custom node help. For easier debugging when both lookups fail, you might wraplastErrorwith the node name and the attempted paths (or a short prefix like"Custom node help not found for ${node.name}: ${lastError}") instead of throwing the bare status text.
51-57: Core help currently has no non-locale fallback unlike custom nodesCustom nodes try both
${locale}.mdand a locale-less.md, but core nodes only attempt/docs/${node.name}/${locale}.md. If your docs layout includes non-localized core markdown, consider mirroring the same fallback pattern here; otherwise the current behavior is fine.
59-80: Harden tryFetchMarkdown against fetch errors and unhelpful HTML status textCentralizing fetch + HTML guarding here is good, but two edge cases are worth tightening:
- If
fetchrejects (network/CORS issues), the promise bypasses this helper’s{ text, errorText }contract and surfaces as a raw exception instead of a structured{ text: null, errorText }.- For HTML fallbacks returning 200,
res.statusTextis typically'OK', so callers can end up throwingError('OK'), which isn’t very actionable.You can keep the existing behavior while normalizing these cases, for example:
private async tryFetchMarkdown( path: string ): Promise<{ text: string | null; errorText?: string }> { - const res = await fetch(api.fileURL(path)) - - if (!res.ok) { - return { text: null, errorText: res.statusText } - } - - const contentType = res.headers?.get?.('content-type') ?? '' - const text = await res.text() - - const isHtmlContentType = contentType.includes('text/html') - - if (isHtmlContentType) return { text: null, errorText: res.statusText } - - return { text } + try { + const res = await fetch(api.fileURL(path)) + + if (!res.ok) { + return { + text: null, + errorText: res.statusText || `HTTP ${res.status}` + } + } + + const contentType = res.headers?.get?.('content-type') ?? '' + const text = await res.text() + + if (contentType.includes('text/html')) { + return { + text: null, + errorText: 'Received HTML while expecting markdown' + } + } + + return { text } + } catch (err) { + const message = + err instanceof Error ? err.message : 'Failed to fetch help markdown' + return { text: null, errorText: message } + } }This keeps the non-throwing contract for normal failures, makes HTML fallbacks self-describing, and ensures network errors are surfaced in a consistent way.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/services/nodeHelpService.ts(2 hunks)src/workbench/utils/nodeHelpUtil.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/workbench/utils/nodeHelpUtil.tssrc/services/nodeHelpService.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/services/nodeHelpService.ts
🧬 Code graph analysis (1)
src/services/nodeHelpService.ts (1)
src/scripts/api.ts (1)
api(1289-1289)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup
🔇 Additional comments (2)
src/workbench/utils/nodeHelpUtil.ts (1)
18-27: Blueprint base URL short-circuit looks correctReturning
''forNodeSourceType.Blueprintprevents bogus/docs/*URLs while leaving CustomNodes/core behavior unchanged. Please just verify allgetNodeHelpBaseUrlcall sites treat an empty string as “no docs base” and don’t blindly append paths to it.src/services/nodeHelpService.ts (1)
10-18: Good early-return for Blueprint node helpShort-circuiting Blueprint nodes to
node.description || ''cleanly avoids unnecessary help fetches and SPA HTML fallbacks while keeping the Custom/Core flows unchanged.
explicitly prevents subgraphs from making an api call since they don't have docs, this was previously reliant on a non-ok resolution
also doesn't try returning anything that has contenttype of text/html to prevent the markdown renderer from crashing
Summary
Testing