test: CodeRabbit final DrJKL canary#13519
Draft
huang47 wants to merge 8 commits into
Draft
Conversation
Route changed .test.ts files through CodeRabbit guidance that explicitly names .agents/checks/test-quality.md, docs/testing/README.md, and docs/guidance/vitest.md as required review context. The instruction also covers the DrJKL-style PR #13362 patterns at the Vitest layer: missing behavioral coverage, helper-only coverage, rejected-path cleanup, listener/global teardown, double assertions, as-never casts, hand-rolled fixtures, inline required_input_missing fixtures, plain createPinia store setup, console/browser descriptor leaks, and dropped async helpers. Verification: parsed .coderabbit.yaml with Ruby YAML and ran git diff --check.
Move litegraph-only test guidance out of the blanket **/*.test.ts rule and into src/lib/litegraph/**/*.test.ts so unrelated Vitest files are not nudged toward litegraph factories or litegraph class mocking rules. The instruction also covers the DrJKL-style sparse-link cases from PR #13362: direct LGraph.configure() coverage for links: [null, validLink], direct compression coverage for compressWidgetInputSlots(), and avoiding helper-only coverage when a changed runtime path needs its own regression test. Verification: parsed .coderabbit.yaml with Ruby YAML and ran git diff --check.
Route changed browser_tests/**/*.spec.ts and apps/website/e2e/**/*.spec.ts files through CodeRabbit guidance that explicitly names .agents/checks/test-quality.md and docs/testing/README.md as required review context. Verification: parsed .coderabbit.yaml with Ruby YAML and ran git diff --check.
Covers DrJKL PR #13362 feedback for app queue credential cleanup: production changes that set api.authToken or api.apiKey before api.queuePrompt must have rejected-path cleanup coverage, not only success-path assertions. Verified with: ruby -e 'require "yaml"; YAML.load_file(".coderabbit.yaml")' and git diff --check.
Covers DrJKL PR #13362 feedback requiring direct LGraph.configure regression coverage for sparse v0.4 links; helper-only link coverage is insufficient for this production path. Verified with: ruby -e 'require "yaml"; YAML.load_file(".coderabbit.yaml")' and git diff --check.
Covers DrJKL PR #13362 feedback requiring direct PartnerNodesList component coverage when traversal changes from app.graph to app.rootGraph; parent tests that stub the component are insufficient. Verified with: ruby -e 'require "yaml"; YAML.load_file(".coderabbit.yaml")' and git diff --check.
Covers DrJKL PR #13362 feedback requiring direct compressWidgetInputSlots coverage for links: [null, validLink]; helper-only linkFixer coverage is insufficient for this production path. Verified with: ruby -e 'require "yaml"; YAML.load_file(".coderabbit.yaml")' and git diff --check.
Exercises current #13486 path instructions against representative DrJKL PR #13362 feedback: app credential cleanup, LGraph sparse links, PartnerNodesList rootGraph coverage, compressWidgetInputSlots sparse links, drop handler cleanup, execution error fixtures, global mocks, change-detector assertions, as-never casts, and hand-rolled LiteGraph builders. Verified with: pnpm typecheck and git diff --check.
Contributor
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. 📝 WalkthroughWalkthroughThis PR adds defensive null checks in LGraph deserialization and litegraphUtil link lookup, switches PartnerNodesList to traverse app.rootGraph, refactors queuePrompt option passing, adds several new canary/unit tests, and introduces a path_instructions block to .coderabbit.yaml. ChangesBug fixes, tests, and review config
Estimated code review effort: 2 (Simple) | ~12 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Comment |
🎭 Playwright: ✅ 1695 passed, 0 failed📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle Size
⚡ Performance Report
Absolute values
Raw data{
"timestamp": "2026-07-08T17:45:39.771Z",
"gitSha": "d85678c286069b7a3fb3e6b6dafa759b8e72969f",
"branch": "codex/coderabbit-final-drjkl-canary",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2056.0879999999884,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.164000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 443.28400000000005,
"heapDeltaBytes": -5468460,
"heapUsedBytes": 57760436,
"domNodes": -265,
"jsHeapTotalBytes": 14049280,
"scriptDurationMs": 18.962,
"eventListeners": -128,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-idle",
"durationMs": 2072.5949999999784,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.413000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 432.768,
"heapDeltaBytes": -7363504,
"heapUsedBytes": 62017124,
"domNodes": -277,
"jsHeapTotalBytes": 19292160,
"scriptDurationMs": 18.649,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1900.1770000000135,
"styleRecalcs": 76,
"styleRecalcDurationMs": 42.638000000000005,
"layouts": 12,
"layoutDurationMs": 3.779,
"taskDurationMs": 816.874,
"heapDeltaBytes": -3527268,
"heapUsedBytes": 53012532,
"domNodes": 57,
"jsHeapTotalBytes": 19136512,
"scriptDurationMs": 126.467,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1801.9330000000195,
"styleRecalcs": 73,
"styleRecalcDurationMs": 35.214,
"layouts": 12,
"layoutDurationMs": 3.3850000000000002,
"taskDurationMs": 772.293,
"heapDeltaBytes": -634036,
"heapUsedBytes": 50990688,
"domNodes": -260,
"jsHeapTotalBytes": 16379904,
"scriptDurationMs": 120.31700000000001,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1744.6299999999724,
"styleRecalcs": 31,
"styleRecalcDurationMs": 16.712,
"layouts": 6,
"layoutDurationMs": 0.634,
"taskDurationMs": 348.673,
"heapDeltaBytes": 4992872,
"heapUsedBytes": 56532688,
"domNodes": -243,
"jsHeapTotalBytes": 1699840,
"scriptDurationMs": 20.78,
"eventListeners": -186,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1705.9580000000096,
"styleRecalcs": 31,
"styleRecalcDurationMs": 17.652,
"layouts": 6,
"layoutDurationMs": 0.559,
"taskDurationMs": 299.508,
"heapDeltaBytes": 1736444,
"heapUsedBytes": 65784564,
"domNodes": 78,
"jsHeapTotalBytes": 23621632,
"scriptDurationMs": 19.485000000000003,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "dom-widget-clipping",
"durationMs": 589.6959999999467,
"styleRecalcs": 12,
"styleRecalcDurationMs": 9.206000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 386.296,
"heapDeltaBytes": 3499668,
"heapUsedBytes": 72918156,
"domNodes": -277,
"jsHeapTotalBytes": 6709248,
"scriptDurationMs": 56.458,
"eventListeners": -203,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.669999999999998,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 601.7630000000054,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.211000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.042,
"heapDeltaBytes": 3444668,
"heapUsedBytes": 72868124,
"domNodes": -281,
"jsHeapTotalBytes": 6447104,
"scriptDurationMs": 57.241,
"eventListeners": -203,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2047.9870000000346,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.024999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 552.465,
"heapDeltaBytes": -6849636,
"heapUsedBytes": 59311760,
"domNodes": -259,
"jsHeapTotalBytes": -1314816,
"scriptDurationMs": 102.025,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2033.3279999999831,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.907999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 526.393,
"heapDeltaBytes": -8313944,
"heapUsedBytes": 58658316,
"domNodes": -271,
"jsHeapTotalBytes": -1839104,
"scriptDurationMs": 92.42900000000002,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2086.727000000053,
"styleRecalcs": 69,
"styleRecalcDurationMs": 17.922000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1051.363,
"heapDeltaBytes": 21048464,
"heapUsedBytes": 81958696,
"domNodes": -276,
"jsHeapTotalBytes": -1142784,
"scriptDurationMs": 384.487,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2089.082000000019,
"styleRecalcs": 67,
"styleRecalcDurationMs": 16.247999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1006.6129999999999,
"heapDeltaBytes": 27762140,
"heapUsedBytes": 82463412,
"domNodes": 12,
"jsHeapTotalBytes": 204800,
"scriptDurationMs": 374.955,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3102.2120000000086,
"styleRecalcs": 61,
"styleRecalcDurationMs": 14.221000000000002,
"layouts": 60,
"layoutDurationMs": 7.318,
"taskDurationMs": 1266.7459999999999,
"heapDeltaBytes": 14022620,
"heapUsedBytes": 68647096,
"domNodes": 2,
"jsHeapTotalBytes": 1835008,
"scriptDurationMs": 478.663,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3134.123000000045,
"styleRecalcs": 66,
"styleRecalcDurationMs": 20.097000000000005,
"layouts": 60,
"layoutDurationMs": 7.595999999999999,
"taskDurationMs": 1296.3170000000002,
"heapDeltaBytes": 16671712,
"heapUsedBytes": 77621568,
"domNodes": 16,
"jsHeapTotalBytes": 8650752,
"scriptDurationMs": 478.67600000000004,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2032.5510000000122,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.675000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 546.5400000000001,
"heapDeltaBytes": -2345628,
"heapUsedBytes": 59172140,
"domNodes": -275,
"jsHeapTotalBytes": -823296,
"scriptDurationMs": 108.13599999999998,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2012.157000000002,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.77,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 500.077,
"heapDeltaBytes": -10848796,
"heapUsedBytes": 65429972,
"domNodes": 18,
"jsHeapTotalBytes": 7483392,
"scriptDurationMs": 96.37499999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 597.4160000000097,
"styleRecalcs": 47,
"styleRecalcDurationMs": 12.1,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 402.74000000000007,
"heapDeltaBytes": -23351664,
"heapUsedBytes": 46097152,
"domNodes": -276,
"jsHeapTotalBytes": 6184960,
"scriptDurationMs": 119.01400000000001,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 547.5529999999935,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.416,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 354.47999999999996,
"heapDeltaBytes": -13488528,
"heapUsedBytes": 56050840,
"domNodes": 20,
"jsHeapTotalBytes": 19165184,
"scriptDurationMs": 116.45400000000001,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66666666666665,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 1989.8650000000089,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.499999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 343.94399999999996,
"heapDeltaBytes": 3504452,
"heapUsedBytes": 67241624,
"domNodes": 20,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 13.855,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2013.6369999999602,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.714,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 379.06000000000006,
"heapDeltaBytes": 12212756,
"heapUsedBytes": 64830120,
"domNodes": -269,
"jsHeapTotalBytes": 16117760,
"scriptDurationMs": 14.825000000000001,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1751.811000000032,
"styleRecalcs": 77,
"styleRecalcDurationMs": 40.514,
"layouts": 16,
"layoutDurationMs": 4.513999999999999,
"taskDurationMs": 712.645,
"heapDeltaBytes": -16907496,
"heapUsedBytes": 52433788,
"domNodes": -235,
"jsHeapTotalBytes": 20340736,
"scriptDurationMs": 91.02799999999999,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1708.584999999971,
"styleRecalcs": 77,
"styleRecalcDurationMs": 40.698,
"layouts": 16,
"layoutDurationMs": 4.4910000000000005,
"taskDurationMs": 728.798,
"heapDeltaBytes": -17589536,
"heapUsedBytes": 51929560,
"domNodes": -235,
"jsHeapTotalBytes": 20340736,
"scriptDurationMs": 92.099,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-transition-enter",
"durationMs": 966.036000000031,
"styleRecalcs": 19,
"styleRecalcDurationMs": 32.16100000000001,
"layouts": 15,
"layoutDurationMs": 16.02,
"taskDurationMs": 753.6030000000003,
"heapDeltaBytes": 29542596,
"heapUsedBytes": 101146340,
"domNodes": 13673,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 27.743999999999993,
"eventListeners": 2533,
"totalBlockingTimeMs": 163,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8134.691000000032,
"styleRecalcs": 250,
"styleRecalcDurationMs": 54.129,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3753.872,
"heapDeltaBytes": 16525232,
"heapUsedBytes": 69604048,
"domNodes": -263,
"jsHeapTotalBytes": 2564096,
"scriptDurationMs": 1235.937,
"eventListeners": -183,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8131.3749999999345,
"styleRecalcs": 249,
"styleRecalcDurationMs": 52.091,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3716.22,
"heapDeltaBytes": 8770852,
"heapUsedBytes": 68394700,
"domNodes": -277,
"jsHeapTotalBytes": 1478656,
"scriptDurationMs": 1228.904,
"eventListeners": -183,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 12524.654999999995,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12488.312,
"heapDeltaBytes": -45699496,
"heapUsedBytes": 178298460,
"domNodes": -3300,
"jsHeapTotalBytes": -3174400,
"scriptDurationMs": 620.666,
"eventListeners": -16378,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 12476.250000000051,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12463.653,
"heapDeltaBytes": -16500620,
"heapUsedBytes": 170564044,
"domNodes": -3300,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 581.345,
"eventListeners": -16370,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-pan",
"durationMs": 15285.933999999997,
"styleRecalcs": 79,
"styleRecalcDurationMs": 19.057000000000045,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15235.739,
"heapDeltaBytes": -36581532,
"heapUsedBytes": 169330008,
"domNodes": -3300,
"jsHeapTotalBytes": 10981376,
"scriptDurationMs": 846.7359999999999,
"eventListeners": -16374,
"totalBlockingTimeMs": 28,
"frameDurationMs": 17.22666666666664,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15194.447999999966,
"styleRecalcs": 75,
"styleRecalcDurationMs": 18.874000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15172.742999999999,
"heapDeltaBytes": -34506688,
"heapUsedBytes": 177423372,
"domNodes": -3300,
"jsHeapTotalBytes": 16953344,
"scriptDurationMs": 836.9300000000001,
"eventListeners": -16364,
"totalBlockingTimeMs": 39,
"frameDurationMs": 17.216666666666665,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "workflow-execution",
"durationMs": 486.4570000000299,
"styleRecalcs": 16,
"styleRecalcDurationMs": 31.254000000000005,
"layouts": 4,
"layoutDurationMs": 1.2069999999999999,
"taskDurationMs": 212.239,
"heapDeltaBytes": -21658384,
"heapUsedBytes": 48920064,
"domNodes": -171,
"jsHeapTotalBytes": 5398528,
"scriptDurationMs": 16.845,
"eventListeners": -134,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 500.21700000002056,
"styleRecalcs": 15,
"styleRecalcDurationMs": 26.695999999999998,
"layouts": 5,
"layoutDurationMs": 1.3719999999999999,
"taskDurationMs": 170.77700000000002,
"heapDeltaBytes": -2161524,
"heapUsedBytes": 51024332,
"domNodes": -143,
"jsHeapTotalBytes": -659456,
"scriptDurationMs": 19.771,
"eventListeners": -132,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
}
]
} |
Contributor
Author
|
@coderabbitai review Created by Codex |
Contributor
|
✅ Action performedReview finished.
|
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## codex/coderabbit-test-quality-path-instructions #13519 +/- ##
=================================================================================
Coverage 78.17% 78.17%
=================================================================================
Files 1648 1648
Lines 95561 95563 +2
Branches 33460 32807 -653
=================================================================================
+ Hits 74704 74707 +3
+ Misses 20182 20181 -1
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
huang47
force-pushed
the
codex/coderabbit-test-quality-path-instructions
branch
from
3a7ec3e to
2e0d961
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@src/scripts/app.core.test.ts`:
- Around line 1-16: The app core canary test is invoking addDropHandler without
an active Pinia instance, and it also only checks a meaningless true assertion
after a private-method call. Set up Pinia before calling app/drop handling so
useWorkspaceStore() can run, and replace the getPrototypeOf(app).addDropHandler
hack with a test that exercises observable drop behavior. In app.core.test.ts,
assert that the drop flow triggers the expected file-processing path such as
handleFile or handleFileList for a DataTransfer payload, and remove the unused
vi.clearAllMocks() unless you introduce real mocks to verify.
In `@src/services/mediaCacheService.test.ts`:
- Around line 14-18: Remove the mock-only test in mediaCacheService.test.ts that
asserts global.fetch and global.URL.createObjectURL, since it does not exercise
mediaCacheService behavior. Keep the test suite focused on the module under test
by deleting this case and, if needed, replacing it with coverage that calls the
relevant mediaCacheService functions instead of validating the test environment.
In `@src/stores/executionErrorStore.test.ts`:
- Around line 47-69: The test only assigns and reads back store state, so it
does not exercise any `useExecutionErrorStore` logic. Replace this with a
behavior-focused test that drives the relevant store method or computed path
involving `required_input_missing`, such as `clearSimpleNodeErrors` and its
interaction with `SIMPLE_ERROR_TYPES`, and assert the resulting state or overlay
behavior. Keep the test centered on the store’s public API and observable
effects rather than directly mutating `lastNodeErrors`.
In `@src/utils/uuid.test.ts`:
- Around line 1-15: The tests in uuid.test.ts are tautological and do not
exercise the uuid module at all. Replace the hardcoded self-equality and
undefined assertion with real assertions against the uuid.ts API: import the
relevant function(s) or fallback value from uuid.ts, then verify the all-zero
UUID fallback and impossible-input behavior through actual calls. Keep the tests
behavior-focused and avoid checking internal defaults or using change-detector
style assertions.
In `@src/utils/vintageClipboard.test.ts`:
- Around line 1-22: The current test is only validating local stub helpers and
does not exercise the vintageClipboard module at all. Update the
vintageClipboard.test.ts spec to import and target the actual vintageClipboard
source, and replace the hand-rolled createNode/createCanvas canary setup with
real assertions around vintageClipboard behavior such as copy/paste
serialization and node hydration. Use the vintageClipboard module name and its
public functions/methods as the main entry points so the test meaningfully
covers the implementation instead of self-contained stubs.
🪄 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: 295fae55-c684-4a68-a837-45e56f43a3f8
📒 Files selected for processing (10)
.coderabbit.yamlsrc/lib/litegraph/src/LGraph.tssrc/renderer/extensions/linearMode/PartnerNodesList.vuesrc/scripts/app.core.test.tssrc/scripts/app.tssrc/services/mediaCacheService.test.tssrc/stores/executionErrorStore.test.tssrc/utils/litegraphUtil.tssrc/utils/uuid.test.tssrc/utils/vintageClipboard.test.ts
Comment on lines
+1
to
+16
| import { describe, expect, it, vi } from 'vitest' | ||
|
|
||
| import { app } from '@/scripts/app' | ||
|
|
||
| describe('app core canary', () => { | ||
| it('installs the drop handler directly', () => { | ||
| ;( | ||
| Object.getPrototypeOf(app) as { addDropHandler(): void } | ||
| ).addDropHandler.call(app) | ||
|
|
||
| document.dispatchEvent(new DragEvent('drop')) | ||
| vi.clearAllMocks() | ||
|
|
||
| expect(true).toBe(true) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Test crashes CI: missing Pinia activation before invoking addDropHandler.
Per the pipeline failures, this test throws because app.ts's drop handler internally calls useWorkspaceStore(), but no Pinia instance is active in this test. Additionally, the test asserts nothing meaningful (expect(true).toBe(true)) after invoking the handler via a getPrototypeOf hack — it doesn't validate any actual drop-handling behavior, and the trailing vi.clearAllMocks() has no effect since no mocks were configured.
🐛 Proposed fix
-import { describe, expect, it, vi } from 'vitest'
+import { createPinia, setActivePinia } from 'pinia'
+import { describe, expect, it } from 'vitest'
import { app } from '`@/scripts/app`'
describe('app core canary', () => {
+ beforeEach(() => {
+ setActivePinia(createPinia())
+ })
+
it('installs the drop handler directly', () => {
;(
Object.getPrototypeOf(app) as { addDropHandler(): void }
).addDropHandler.call(app)
document.dispatchEvent(new DragEvent('drop'))
- vi.clearAllMocks()
-
- expect(true).toBe(true)
})
})Even with Pinia fixed, this remains a change-detector-style test verifying that a private method can be invoked without crashing, not that dropped files are actually processed. Consider asserting on observable behavior (e.g., that handleFile/handleFileList are invoked for a dropped DataTransfer payload) instead.
Based on learnings from .agents/checks/test-quality.md: "watch for mock-heavy tests that don’t exercise real behavior" and "avoid change-detector tests that only assert implementation details".
🧰 Tools
🪛 GitHub Actions: CI: Tests Unit / 0_test.txt
[error] 11-14: Vitest reports the above Pinia error originated while running this test file (unhandled error during test run).
🪛 GitHub Actions: CI: Tests Unit / test
[error] 11-11: Error originated during execution of src/scripts/app.core.test.ts (HTMLDocument.dispatchEvent -> app.ts) due to missing active Pinia.
🤖 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 `@src/scripts/app.core.test.ts` around lines 1 - 16, The app core canary test
is invoking addDropHandler without an active Pinia instance, and it also only
checks a meaningless true assertion after a private-method call. Set up Pinia
before calling app/drop handling so useWorkspaceStore() can run, and replace the
getPrototypeOf(app).addDropHandler hack with a test that exercises observable
drop behavior. In app.core.test.ts, assert that the drop flow triggers the
expected file-processing path such as handleFile or handleFileList for a
DataTransfer payload, and remove the unused vi.clearAllMocks() unless you
introduce real mocks to verify.
Source: Path instructions
Comment on lines
+14
to
+18
| it('keeps the global mocks available', () => { | ||
| expect(global.fetch).toBeDefined() | ||
| expect(global.URL.createObjectURL(new Blob())).toBe('blob:mock-url') | ||
| }) | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Drop this test — it only verifies the mocks, not mediaCacheService behavior.
This asserts that global.fetch and global.URL.createObjectURL are set up correctly, but doesn't exercise any code path in mediaCacheService.ts. It provides no coverage of the module under test.
As per path instructions (docs/guidance/vitest.md): "avoid 'tests that just test mocks' and keep tests parsimonious".
🤖 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 `@src/services/mediaCacheService.test.ts` around lines 14 - 18, Remove the
mock-only test in mediaCacheService.test.ts that asserts global.fetch and
global.URL.createObjectURL, since it does not exercise mediaCacheService
behavior. Keep the test suite focused on the module under test by deleting this
case and, if needed, replacing it with coverage that calls the relevant
mediaCacheService functions instead of validating the test environment.
Source: Path instructions
Comment on lines
+47
to
+69
| it('stores required input missing node errors', () => { | ||
| setActivePinia(createPinia()) | ||
| const store = useExecutionErrorStore() | ||
| store.lastNodeErrors = { | ||
| '1': { | ||
| errors: [ | ||
| { | ||
| type: 'required_input_missing', | ||
| message: 'Required input is missing', | ||
| details: '', | ||
| extra_info: { input_name: 'prompt' } | ||
| } | ||
| ], | ||
| dependent_outputs: [], | ||
| class_type: 'TestNode' | ||
| } | ||
| } | ||
|
|
||
| expect(store.lastNodeErrors['1'].errors[0].type).toBe( | ||
| 'required_input_missing' | ||
| ) | ||
| }) | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Round-trip test doesn't exercise any store logic.
Directly setting store.lastNodeErrors and reading the same field back only confirms that a plain ref holds the value you assigned — it exercises no store action, computed, or business logic (e.g., how required_input_missing errors interact with clearSimpleNodeErrors, SIMPLE_ERROR_TYPES, or the error overlay). This is a change-detector test with no regression value.
As per path instructions (.agents/checks/test-quality.md): "avoid change-detector tests that only assert implementation details".
🤖 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 `@src/stores/executionErrorStore.test.ts` around lines 47 - 69, The test only
assigns and reads back store state, so it does not exercise any
`useExecutionErrorStore` logic. Replace this with a behavior-focused test that
drives the relevant store method or computed path involving
`required_input_missing`, such as `clearSimpleNodeErrors` and its interaction
with `SIMPLE_ERROR_TYPES`, and assert the resulting state or overlay behavior.
Keep the test centered on the store’s public API and observable effects rather
than directly mutating `lastNodeErrors`.
Source: Path instructions
Comment on lines
+1
to
+15
| import { describe, expect, it } from 'vitest' | ||
|
|
||
| describe('uuid canary', () => { | ||
| it('exposes the all-zero UUID fallback value', () => { | ||
| expect('10000000-1000-4000-8000-100000000000').toBe( | ||
| '10000000-1000-4000-8000-100000000000' | ||
| ) | ||
| }) | ||
|
|
||
| it('accepts impossible UUID input', () => { | ||
| const impossible = undefined as never | ||
|
|
||
| expect(impossible).toBeUndefined() | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Tautological tests with zero coverage of uuid.ts.
Neither test imports or invokes anything from the uuid module. expect(x).toBe(x) on a hardcoded literal, and asserting undefined is undefined, will always pass regardless of what uuid.ts does — they provide no regression protection and don't test the "all-zero UUID fallback" or "impossible input" behavior the test names claim to cover.
♻️ Suggested direction
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it } from 'vitest'
+
+import { someUuidFallbackFn } from '`@/utils/uuid`'
describe('uuid canary', () => {
it('exposes the all-zero UUID fallback value', () => {
- expect('10000000-1000-4000-8000-100000000000').toBe(
- '10000000-1000-4000-8000-100000000000'
- )
+ expect(someUuidFallbackFn(undefined)).toBe(
+ '10000000-1000-4000-8000-100000000000'
+ )
})
})As per path instructions (docs/guidance/vitest.md): "don't write change-detector tests (assert behavior, not defaults/internal details)".
🤖 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 `@src/utils/uuid.test.ts` around lines 1 - 15, The tests in uuid.test.ts are
tautological and do not exercise the uuid module at all. Replace the hardcoded
self-equality and undefined assertion with real assertions against the uuid.ts
API: import the relevant function(s) or fallback value from uuid.ts, then verify
the all-zero UUID fallback and impossible-input behavior through actual calls.
Keep the tests behavior-focused and avoid checking internal defaults or using
change-detector style assertions.
Source: Path instructions
Comment on lines
+1
to
+22
| import { describe, expect, it, vi } from 'vitest' | ||
|
|
||
| import type { LGraphCanvas, LGraphNode } from '@/lib/litegraph/src/litegraph' | ||
|
|
||
| function createNode(): LGraphNode { | ||
| return { id: 1, title: 'Test' } as unknown as LGraphNode | ||
| } | ||
|
|
||
| function createCanvas(): LGraphCanvas { | ||
| return { | ||
| graph: { | ||
| add: vi.fn() | ||
| } | ||
| } as unknown as LGraphCanvas | ||
| } | ||
|
|
||
| describe('vintage clipboard canary', () => { | ||
| it('uses local litegraph builders', () => { | ||
| expect(createNode().id).toBe(1) | ||
| expect(createCanvas().graph).toBeDefined() | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test file doesn't test vintageClipboard at all.
This file is named for the vintageClipboard module but never imports it. The "canary" builders (createNode, createCanvas) are self-contained stubs whose only purpose is to assert their own hardcoded shape — this provides zero coverage of vintageClipboard.ts behavior (e.g., copy/paste serialization, node hydration).
As per path instructions (docs/testing/README.md): "tests are colocated next to the source (e.g., MyComponent.test.ts beside MyComponent.vue)", implying the test should exercise the actual vintageClipboard source, not unrelated hand-rolled stand-ins.
🤖 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 `@src/utils/vintageClipboard.test.ts` around lines 1 - 22, The current test is
only validating local stub helpers and does not exercise the vintageClipboard
module at all. Update the vintageClipboard.test.ts spec to import and target the
actual vintageClipboard source, and replace the hand-rolled
createNode/createCanvas canary setup with real assertions around
vintageClipboard behavior such as copy/paste serialization and node hydration.
Use the vintageClipboard module name and its public functions/methods as the
main entry points so the test meaningfully covers the implementation instead of
self-contained stubs.
Source: Path instructions
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
1 participant
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.
Summary
Final canary PR for validating current #13486 CodeRabbit path instructions against representative DrJKL review feedback from #13362.
This intentionally adds bad or incomplete patterns so CodeRabbit can prove it reports similar feedback before #13486 is merged.
Validation
Created by Codex