Skip to content

Fix editor image store mocks after import changes - #25333

Closed
nwparker wants to merge 3 commits into
mainfrom
orca/editor-image-insertion-test-callback
Closed

nwparker wants to merge 3 commits into
mainfrom
orca/editor-image-insertion-test-callback

Conversation

@nwparker

@nwparker nwparker commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​19 $\color{#cf222e}{\Huge{\mathbf{−}}}$​4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​15
Prod 0 0 0 0

ELI5

Three editor image test files on main have incomplete store mocks. Image imports fail before running, so 38 tests fail and block other pull requests.

What Changed

Add the existing openFiles field to three editor store mocks, so image imports can inspect the active file. Remove an unnecessary editor cast in the outside-project import test.

Why

These tests need the same inputs as the editor. Updating the fixtures preserves all existing import, access, insertion, table and cancellation assertions and requires no application change.

Linked Issue

Follows #24489 and @Jinwoo-H’s callback correction in #25351; unblocks #24768 and #25326.

Visual Proof

N/A — only four test fixtures change.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

On macOS, the full web typecheck passes with dependencies matching the lockfile. Reproduced all 38 failures from the three failed CI shards locally: 19 passed before the mock correction, and all 57 pass afterward. The separate seven image-insertion tests also pass. Lint, formatting and all changed-code quality checks pass for the complete four-file change.

Review

Independently reviewed the store access and the exact mock-only correction. No assertions were removed or weakened. The original typecheck failure is already corrected on main by #25351; the local tests qualify the remaining mock changes.

Agent skill upstream boundary

  • Not applicable

Notes

The existing tests cover local files, project files, and SSH-owned files. This change has no application, platform, or remote protocol impact. CI covers the broader repository checks; no application build is needed for these fixture corrections.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f8ae18fd-a66c-47c0-8562-c3c33dfcdc63
📥 Commits

Reviewing files that changed from the base of the PR and between bcd6bdc and 0d4728c.

📒 Files selected for processing (1)
  • src/renderer/src/components/editor/rich-markdown-image-insert.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The image-insert test passes the editor directly to the insertion call instead of casting it to never. Three other editor tests add openFiles: [] to their mocked store state.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 0d472

The changes are limited to editor test setup, with no remaining merge-blocking concern.

Architecture Summary

Architecture risk: 🔵 Low · up to 0d472

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (ui) was modified; 4 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in src/renderer/src/components/editor/rich-markdown-async-image-paste.test.ts: The mocked store state now includes openFiles: []; the previous mock omitted this property.
  • observed — Modified behavior in src/renderer/src/components/editor/rich-markdown-async-image-table.test.ts: The mocked store state adds openFiles: []; the existing settings, folderWorkspaces, and worktreesByRepo values remain.
  • observed — Modified behavior in src/renderer/src/components/editor/useLocalImagePick.test.tsx: The mocked store state adds an empty openFiles array; the existing settings, folder-workspace, and worktree values remain.
  • observed — Modified behavior in src/renderer/src/components/editor/rich-markdown-image-insert.test.ts: The test passes editor directly to the insertion call, removing the as never cast.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: updating editor image store mocks after import changes.
Description check ✅ Passed The description covers the required sections, explains the test-fixture changes and rationale, identifies related issues, states why visual proof is not applicable, and reports testing results.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • Current insertion-range callback passed in the outside-project cases — swaps the removed insertPos: 4 for getInsertionRange: () => ({ from: 4, to: 4, requestOrder: 1 }), matching RichMarkdownImageInsertionRange and the shape already used by the other cases in this file.
  • Unnecessary cast dropped — the editorWithRunResult helper already returns an Editor, so editor as never becomes editor against the editor: Editor parameter.

Test-only fixture correction that restores typechecking after the API migration; no assertions or production code change. The collapsed from === to === 4 range preserves the previous insertion position, so the existing insertContentAt and access assertions still pin real behavior. Verified locally: pnpm test src/renderer/src/components/editor/rich-markdown-image-insert.test.ts (7/7 pass) and pnpm tc:web both pass.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@nwparker nwparker changed the title Fix image-insertion test arguments after API migration Fix editor image test fixtures after import changes Oct 5, 2026

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (975c0c5563) — commit bcd6bdc560, which adds the missing openFiles field to three editor store mocks.

  • openFiles added to three store mocks — rich-markdown-async-image-paste.test.ts, rich-markdown-async-image-table.test.ts, and useLocalImagePick.test.tsx now expose openFiles: [] from the mocked useAppStore.getState(), matching the field insertRichMarkdownImageFromPath reads at rich-markdown-image-insert.ts:74.

The three shards failed because the mock omitted openFiles, so state.openFiles.find(...) threw; the field addition is the minimal fixture correction and no assertion is changed or weakened. Verified locally: all four affected files pass (64 tests).

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@nwparker nwparker changed the title Fix editor image test fixtures after import changes Fix editor image store mocks after import changes Oct 5, 2026
@nwparker

nwparker commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

The two CI repairs are now on main through @Jinwoo-H’s #25351 and #25357. I checked main at d9846e64feaed910b058c4128eb4d46ed8e2c4c8 against this PR’s tested source: the three store mocks differ only in property order, and the insertion test differs only in the existing editor as never cast. This PR’s CI is also green (13 successful checks, 18 skipped). Closing this duplicate; the OpenCode follow-up PRs can now use the repaired main.

@nwparker nwparker closed this Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant