src/platform/remote/comfyui/jobs/fetchJobs.ts (1)

68-70: 💤 Low value

Consider including status text in error message for easier debugging.

Adding res.statusText would provide more context when investigating failures (e.g., distinguishing "400 Bad Request" from "400 INVALID_CURSOR").

♻️ Suggested improvement

   if (!res.ok) {
-    throw new Error(`[Jobs API] Failed to fetch jobs: ${res.status}`)
+    throw new Error(`[Jobs API] Failed to fetch jobs: ${res.status} ${res.statusText}`)
   }

🤖 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/platform/remote/comfyui/jobs/fetchJobs.ts` around lines 68 - 70, Update
the error thrown on non-OK responses in the fetchJobs flow so it includes both
res.status and res.statusText for better context; locate the check that throws
new Error(`[Jobs API] Failed to fetch jobs: ${res.status}`) (in
src/platform/remote/comfyui/jobs/fetchJobs.ts / the fetchJobs handler) and
change the message to include res.statusText (e.g., combine status and
statusText into the thrown Error message) so failures show both numeric code and
human-readable status.

src/views/GraphView.test.ts (1)

103-107: ⚡ Quick win

Add behavioral assertions for the new history-refresh path.

This mock shape update is necessary, but the suite still doesn’t verify that
status / execution_success flows call assetsStore.refreshHistoryHead()
under the assets-open or linear-mode conditions introduced in GraphView.vue.
Please add focused event-path tests so this integration can’t regress silently.

As per coding guidelines, src/**/*.test.ts should “write tests for all
changes, especially bug fixes to catch future regressions” and aim for
behavioral coverage of critical/new features.

🤖 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/views/GraphView.test.ts` around lines 103 - 107, The test suite currently
mocks useAssetsStore but lacks assertions that the new history-refresh code path
in GraphView.vue calls assetsStore.refreshHistoryHead; add focused behavioral
tests in GraphView.test.ts that simulate the two event flows (status change and
execution_success) under the two relevant conditions (assets-open and
linear-mode) and assert that the mocked refreshHistoryHead() is called (and not
called when conditions are false). Locate and extend the existing mocked
useAssetsStore (updateHistory and refreshHistoryHead) and add tests that trigger
the component events or lifecycle/state changes used by GraphView (e.g.,
emitting the status/execution_success events or toggling the
assets-open/linear-mode flags) and verify refreshHistoryHead was invoked the
expected number of times.

Source: Coding guidelines

src/stores/assetsStore.test.ts (1)

30-47: 💤 Low value

Consider using a function declaration for the helper.

Per coding guidelines, prefer function declarations over function expressions for pure functions. This helper could be declared as:

-const mockHistoryPage = (
-  jobs: JobListItem[],
-  {
-    hasMore = false,
-    nextCursor,
-    offset = 0,
-    total = jobs.length,
-    limit = 200
-  }: Partial<Omit<FetchHistoryPageResult, 'jobs'>> = {}
-): FetchHistoryPageResult => ({
-  jobs,
-  total,
-  offset,
-  limit,
-  hasMore,
-  nextCursor
-})
+function mockHistoryPage(
+  jobs: JobListItem[],
+  {
+    hasMore = false,
+    nextCursor,
+    offset = 0,
+    total = jobs.length,
+    limit = 200
+  }: Partial<Omit<FetchHistoryPageResult, 'jobs'>> = {}
+): FetchHistoryPageResult {
+  return {
+    jobs,
+    total,
+    offset,
+    limit,
+    hasMore,
+    nextCursor
+  }
+}

Based on learnings: "Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository."

🤖 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/assetsStore.test.ts` around lines 30 - 47, The helper is defined
as a const arrow function (mockHistoryPage) but the repo prefers pure function
declarations; change the arrow function to a named function declaration
(function mockHistoryPage(jobs: JobListItem[], opts:
Partial<Omit<FetchHistoryPageResult, 'jobs'>> = {}) : FetchHistoryPageResult)
preserving the same parameter defaults and return structure (jobs, total,
offset, limit, hasMore, nextCursor) and keep types JobListItem and
FetchHistoryPageResult intact so callers/tests remain unchanged.

Source: Learnings

src/stores/assetsStore.ts (2)

157-178: 💤 Low value

Consider binary search for sorted insertion.

The current implementation uses findIndex (O(n)) for each insertion, resulting in O(n·m) complexity. Since the list maintains sorted order by created_at, a binary search would reduce this to O(m·log n). Additionally, the repeated new Date() calls inside findIndex create many short-lived objects.

This is acceptable given the MAX_HISTORY_ITEMS=1000 bound, but could be optimized if refresh latency becomes noticeable.

🤖 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/assetsStore.ts` around lines 157 - 178, mergeHistoryAssets
currently uses findIndex with repeated new Date() calls, giving O(n·m) behavior
and many short-lived Date objects; replace the linear search with a binary
search over allHistoryItems.value (which is sorted by created_at) to compute
insertIndex in O(log n), and convert each asset.created_at and existing
item.created_at to timestamps once (e.g., const assetTime = +new
Date(asset.created_at ?? 0)) to avoid repeated Date construction; keep skipping
duplicates via loadedIds and maintain the same push/splice logic (use the
computed insertIndex === -1 meaning append) and respect MAX_HISTORY_ITEMS if
applicable.

---

335-335: Fix: job.id matches asset.id for this overlap check

loadedIds is populated with AssetItem.id from mapHistoryToAssets, and mapTaskOutputToAssetItem sets id: taskItem.jobId; TaskItemImpl.jobId is derived directly from job.id, so page.jobs.some(job => loadedIds.has(job.id)) compares the same id space.

The only reason reachesLoadedItems could be false despite overlap is filtering: mapHistoryToAssets only adds completed jobs that have preview_output, while page.jobs includes other terminal statuses/jobs.

🤖 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/assetsStore.ts` at line 335, reachesLoadedItems is false due to
mapHistoryToAssets only adding AssetItems for completed jobs with
preview_output, so loadedIds lacks IDs for other terminal jobs present in
page.jobs; update mapHistoryToAssets (and/or mapTaskOutputToAssetItem where
AssetItem.id is set) to also include terminal jobs from history even if they
lack preview_output (or otherwise ensure loadedIds includes job.id for those
terminal statuses), so page.jobs.some(job => loadedIds.has(job.id)) correctly
detects overlap with TaskItemImpl.jobId/job.id.