fix: resolve three verified bugs from #1246

[chhoumann]

Fixes the suspected bugs from #1246 that we empirically verified as real — each was reproduced AND survived an independent refutation pass. The other three were verified as not real and left unchanged.

Fixed (3, low severity)

• choiceService.duplicateChoice dropped fields for Multi choices — the Multi branch skipped the excludeKeys copy the other types use, so duplicating a Multi lost command/onePageInput (a Multi with a registered Obsidian command lost it on copy). Now copies all non-id/name/choices fields, then recursively duplicates children with fresh ids.
• Unreachable package version error — isQuickAddPackage required schemaVersion === current exactly, so a too-new package failed with the generic "not a valid package" error and parseQuickAddPackage's specific "newer than this plugin supports" message never fired. Relaxed the guard to a structural check (integer >= 1); parseQuickAddPackage now owns the version-range check. Widened schemaVersion to number.
• quickAddApi.utility.getSelectedText returned undefined on no-view/no-selection while the sibling getSelection returns "" — so getSelectedText().toUpperCase() could throw in user scripts. Now returns "".

Verified NOT bugs (left unchanged)

• executeChoice "proceeds with undefined choice" — the if (!choice) branch is unreachable: plugin.getChoiceByName throws on a miss and is typed : IChoice. (This was the one I'd flagged "medium" — the refutation pass caught that stage-1's reproduction assumed the wrong getChoiceByName contract.)
• Import cycle-guard bookkeeping — proven harmless via a 5000-trial differential test (zero observable difference).
• Export asset-kind dedup precedence — intended; the export/import round-trip is lossless either way.

Verification

bun run test (1400 pass), bun run build-with-lint clean, bun run check (svelte-check) clean. Each fix has a test asserting the corrected behavior.

---

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling in text selection to return consistent empty string values
  • Enhanced schema version validation with clearer error messaging for incompatible packages

• Improvements

  • Duplicating Multi choices now preserves additional properties (command, onePageInput)
  • Package validation now supports forward compatibility with future schema versions

[cloudflare-workers-and-pages]

Deploying quickadd with    Cloudflare Pages

Latest commit:	941ec1a
Status:	✅  Deploy successful!
Preview URL:	https://fa8d2195.quickadd.pages.dev
Branch Preview URL:	https://fix-verified-bugs-1246.quickadd.pages.dev

View logs

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb9f3c1f-cadb-40c3-bb49-f133a308aec7

📥 Commits

Reviewing files that changed from the base of the PR and between e9a9a06 and 941ec1a.

📒 Files selected for processing (6)

• src/quickAddApi.test.ts
• src/quickAddApi.ts
• src/services/choiceService.test.ts
• src/services/choiceService.ts
• src/services/packageImportService.test.ts
• src/types/packages/QuickAddPackage.ts

---

📝 Walkthrough

Walkthrough

This PR updates three distinct behaviors: utility.getSelectedText now returns empty string instead of undefined on errors while preserving error reporting, duplicateChoice preserves all top-level Multi properties via Object.assign instead of selective copying, and QuickAddPackage schema validation relaxes from exact version equality to a supported integer range check.

Changes

Error handling, choice duplication, and schema version validation updates

Layer / File(s)	Summary
getSelectedText return value contract src/quickAddApi.ts, src/quickAddApi.test.ts	utility.getSelectedText now returns "" instead of undefined in error paths (no active view, nothing selected) while still reporting errors; test expectations updated to match.
Multi-choice duplication property preservation src/services/choiceService.ts, src/services/choiceService.test.ts	duplicateChoice preserves all top-level Multi properties except id, name, and choices via Object.assign, replacing selective manual copying; test verifies command and onePageInput are preserved and child choices get fresh ids.
Schema version validation decoupling src/types/packages/QuickAddPackage.ts, src/services/packageImportService.test.ts	QuickAddPackage.schemaVersion type changes from literal constant to generic number, and isQuickAddPackage validation shifts from exact equality to structural check (integer >= 1); test assertion updated to expect specific error message for unsupported versions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• #1246: Directly related; implements the same code-level fixes for getSelectedText, duplicateChoice, and QuickAddPackage schema validation that were reported in the issue.

Suggested labels

released

Poem

🐰 Strings now graceful, returning empty instead of void,
Choices duplicate with care, no properties destroyed,
Schemas grow flexible, versions find their way,
A refactor both modest and refined today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change—fixing three verified bugs from issue #1246—and is concise, specific, and directly related to the changeset.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/verified-bugs-1246

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.