fix: a malformed folder in data.json no longer takes the plugin down with it

[chhoumann]

Fixes #1566.

The problem is bigger than the title

The issue says a malformed folder blanks the settings tab. It does — but that is the second thing that goes wrong. Reproducing it in an isolated Obsidian 1.13 vault showed the plugin never finishes loading at all:

TypeError: t is not iterable
  at addCommandsForChoices (plugin:quickadd)
  at addCommandForChoice   (plugin:quickadd)

addCommandForChoice runs straight from onload(), so a single folder in data.json with no choices key cost:

• every choice command (the palette loses all of them)
• migrations — never ran
• the QuickAdd CLI handlers — never registered, so quickadd:list was "command not found"
• startup macros — never ran
• the update announcement

Obsidian reported only "Plugin failure: quickadd". The settings tab had been registered a few lines earlier, which is the only reason the bug looks like "the settings tab is blank".

Before — the whole tab, not just one row (the other eight setting groups are gone too):

The real defect

IMultiChoice.choices: IChoice[] is an invariant the code assumes and nothing enforces. data.json is untrusted input: hand-edits, imported packages, Obsidian Sync's whole-file last-write-wins, partial writes. An audit found ~40 readers, each deciding for itself whether to guard — with two guards that look right and are not:

choice.choices ?? []      // {} is not nullish → passes straight through
if (choice.choices) {}    // {} is truthy      → passes straight through

Only Array.isArray rejects both shapes. So this is a function, not a convention:

helper	role
childChoicesOf(choice)	READ view. Always safe to iterate/map/spread. Never repairs.
hasChildChoices(choice)	WRITE guard. May this node be rebuilt?
hasUnreadableChildren(c)	Is the value lossy, or genuinely empty?
rootChoicesOf(value)	The same, one level up, for settings.choices.
isChoiceLike(value)	A list entry can be null or a primitive.

The part that was easy to get wrong

dedupeChoicesById deliberately preserves a malformed folder rather than fabricating []. Three independent reviewers converged on the same blocking objection to the obvious fix: updateMultiById and updateChoiceHelper re-spread every folder they walk past — not just the one they target — and both are on the save path. Reading their children through the total accessor would have persisted [] over the preserved value the first time the user collapsed an unrelated folder. Exactly the data loss the preservation stance exists to prevent, triggered by an innocuous click instead of by load.

So the rule the change turns on:

Reads go through the accessor. A walker that rebuilds a node it is not targeting returns it untouched.

Every case in choiceService.malformed.test.ts asserts the malformed values are byte-identical afterwards, not merely that nothing threw. That second assertion is what fails on the naive fix — I verified it does by reverting each guard in turn.

What the user sees now

A folder that lost nothing — no key, null, {} — renders as an ordinary empty folder, with the "Empty — add a choice or drag one here." hint from #1569. It can be renamed, moved, deleted, and refilled; dropping a choice in repairs the value.

A folder whose value could still be holding choices ({"0": {...}}) is the one case where "empty" would be a lie. It says so, and offers neither the drop zone nor the add row — the two affordances that would overwrite it.

The same predicate drives the hint, the picker's row marker and the delete confirmation, so the three surfaces cannot disagree about the same folder:

A corrupt root choices gets an error card rather than the "No choices yet" hero — whose single CTA would write a fresh list straight over it. The rest of the settings tab still renders:

Tradeoffs

Read-layer fix, not load-time normalization. Normalizing choices to [] in loadSettings would be a one-file change and make every downstream reader correct for free. Rejected: the next ordinary save would persist that [] over a value the user may still be able to recover by hand, and QuickAdd offers no other way to get it back. The read layer keeps the file untouched. The cost is a wider diff, which the sweep test exists to make safe.

IMultiChoice.choices is now optional. strictNullChecks is on and CI runs svelte-check, so the missing-key shape is a compile error at every unguarded read, in .ts and .svelte alike. It cost 14 source sites (all of which needed converting anyway) and some ! in test assertions. The non-array shape can't be expressed in the type system, which is why the accessor still has to exist.

onload fails open. Each choice-dependent step is individually guarded, and addCommandsForChoices isolates per choice. The accessors handle the corrupt shapes we know about; this bounds the blast radius of the ones nobody has thought of yet. One defect should cost one command, not the plugin.

No retry button on the error card. A retry re-renders the same data and throws again, and anything that could rewrite data.json would destroy the value the user needs. The card names the file and says QuickAdd has not touched it.

Migrations that can't finish stay pending. removeMacroIndirection moves legacy macros into the choice tree and then deletes the old array — with an unreadable root there is nowhere to move them, so finishing would delete them outright, permanently, since migrations are flagged once and never retried. It returns { complete: false } instead. (An adversarial reviewer caught this as a regression this branch introduced; before the fix the throw at least left the data intact.)

What stops the 41st reader

src/utils/malformedChoices.entrypoints.test.ts: one fixture tree carrying every malformed shape at every depth, pushed through every exported walker, asserting both no-throw and blob equality. Adding a new walker costs one line.

It earned its keep twice. The first version only placed corruption at the root, and a reviewer pointed out that a walker which guards its entry point but not its recursion passes that happily. Nesting the fixture immediately caught duplicateChoice, packageTraversal, and the package-import writers — which fabricated [] over an unreadable value, the exact invariant the change turns on.

Validation

4189 tests, tsc, svelte-check and eslint all clean. main.ts cannot be imported under vitest (it pulls in obsidian), so its half is proven live rather than by unit test.

Live, in an isolated Obsidian 1.13 vault seeded with a folder missing the key, one with {}, one with {"0": {...a real choice...}}, and a null list entry:

check	result
plugin load	no "Plugin failure", 3 choice commands registered
quickadd:list	ok:true, all 7 choices, malformed folders listed as empty
migrations	15 complete, zero console errors
settings tab	all 8 groups, 7 rows, 1 unreadable-folder hint
collapse + duplicate + rename	all three malformed values byte-identical in data.json
add into the {} folder	repaired to a real array; the lossy one untouched
delete the lossy folder	confirmation warns first
run a folder from a command	honest notice, not a dead picker
corrupt root	error card, rest of the tab alive, data.json md5-identical after open/close

Reviewed

An ultracode design review (3 audit passes + 3 adversarial critiques + synthesis) reshaped the design before any code was written — the read/write split, the [null] load-path bug, and the root-corruption case all came from it. A second adversarial pass over the implementation produced 26 findings, 18 of which survived independent verification and are fixed here, including the blocking migration regression above.

Filed separately, not grown into this diff

• An error boundary at the mountComponent seam for every Svelte host (modals, package manager), which needs a per-host fallback story.
• ChoiceView's async row actions (delete, duplicate, configure) degrade to a silent no-op on throw; they should route through reportError.
• data.json schema validation with quarantine-and-backup on load — the general answer this deliberately does not build.

Summary by CodeRabbit

• Bug Fixes
  • Improved robustness across the settings UI, command/URI handling, and editing flows when choice trees are malformed, include holes, or contain unreadable folders.
  • Prevented accidental overwrites by preserving unreadable sections and avoiding rendering/editing gaps that broke drag-and-drop or reordering.
  • Enhanced empty vs unreadable folder messaging, counts, and drill-down navigation.
• New Features
  • Added an “unavailable choices” screen when choices can’t be safely read.
• Migrations
  • Migrations now remain pending when the choices tree is unreadable instead of rewriting it.
• Tests
  • Expanded coverage for malformed traversal and write/read protections across key entry points and services.

[cloudflare-workers-and-pages]

Deploying quickadd with    Cloudflare Pages

Latest commit:	abb1b0c
Status:	✅  Deploy successful!
Preview URL:	https://a8ac6d3d.quickadd.pages.dev
Branch Preview URL:	https://chhoumann-1566-settings-resi.quickadd.pages.dev

View logs

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

QuickAdd now tolerates malformed choice trees across settings rendering, tree edits, package operations, command registration, startup, and migrations. Shared traversal guards preserve unreadable values, distinguish empty from unreadable folders, and prevent malformed data from blanking the settings tab or being replaced with fabricated arrays.

Changes

Malformed choice-tree resilience

Layer / File(s)	Summary
Safe traversal contracts and fixtures src/types/choices/IMultiChoice.ts, src/utils/choiceUtils.ts, src/utils/malformedChoices.*, src/utils/*.test.ts	Traversal guards handle malformed roots, holes, missing children, and unreadable folder contents while preserving malformed values.
Choice editing and package traversal src/services/choiceService.ts, src/services/packageImportService.ts, src/utils/packageTraversal.ts, src/services/*malformed.test.ts	Tree edits and package operations skip invalid nodes, preserve unreadable folders, and repair only safely writable malformed folders.
Settings rendering and folder interaction src/gui/choiceList/*, src/gui/suggesters/*, src/gui/choiceList/contextMenu.ts, src/choiceExecutor.ts	Settings views and suggesters distinguish empty from unreadable folders and render unavailable states instead of failing.
Application and integration entry points src/main.ts, src/quickAddSettingsTab.ts, src/cli/*, src/gui/MacroGUIs/MacroBuilder.ts, src/gui/PackageManager/ExportPackageModal.svelte	Startup, mounting, command registration, CLI flattening, and package traversal use guarded helpers and error isolation.
Migration preservation and pending results src/migrations/*	Migrations leave unreadable roots untouched and return incomplete results instead of persisting fabricated empty arrays.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SettingsTab
  participant ChoiceView
  participant ChoiceUtils
  participant ChoiceService
  participant SettingsStore
  SettingsTab->>ChoiceView: mount choices
  ChoiceView->>ChoiceUtils: read root and child choices
  ChoiceUtils-->>ChoiceView: readable choices or unavailable state
  ChoiceView->>ChoiceService: apply a tree edit
  ChoiceService->>ChoiceUtils: validate readable children
  ChoiceService-->>SettingsStore: preserve or update choice tree

Loading

Possibly related PRs

• chhoumann/quickadd#1550: Updates the same empty-folder notice paths in the suggester and choice executor.
• chhoumann/quickadd#1569: Overlaps with empty-folder detection and activation handling in choiceSuggester.
• chhoumann/quickadd#1457: Aligns with migration pending-result handling introduced for unreadable choice data.

Suggested labels: released

Poem

I’m a rabbit in the data tree,
Guarding folders carefully.
Holes may hop and roots may bend,
No lost choices at the end.
Empty flags and warnings glow—
Safe burrows help QuickAdd grow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 61.70% 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 clearly matches the main change: hardening QuickAdd against malformed folder data in data.json.
Linked Issues check	✅ Passed	The PR guards the settings tree against malformed Multi choices and preserves deletion/rendering behavior, which directly fixes #1566.
Out of Scope Changes check	✅ Passed	The broader startup, migration, and service changes all support the same malformed-data resilience goal and don't look unrelated.

✨ 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 chhoumann/1566-settings-resilience

---

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.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b5e72277b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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]

Actionable comments posted: 1

🧹 Nitpick comments (4)

src/migrations/helpers/isCaptureChoice.ts (1)

5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Parameter typed IChoice despite being explicitly null/primitive-tolerant.

Sibling predicate isMultiChoice(choice: unknown) types its input as unknown to reflect that it must handle malformed data; this function keeps choice: IChoice even though the comment states it must tolerate null/stray primitives. Widening to unknown would make the signature honest and let TS flag misuse at other call sites.

♻️ Suggested signature

-export function isCaptureChoice(choice: IChoice): choice is CaptureChoice {
+export function isCaptureChoice(choice: unknown): choice is CaptureChoice {

🤖 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/migrations/helpers/isCaptureChoice.ts` around lines 5 - 9, Update the
isCaptureChoice parameter type from IChoice to unknown, matching its null- and
primitive-tolerant behavior and the sibling isMultiChoice predicate. Keep the
existing isChoiceLike and Capture type-guard logic unchanged.

src/quickAddSettingsTab.ts (1)

423-443: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

mountSettingView's props type is decoupled from the actual component argument.

mountComponent ties props: ComponentProps<C> to the same generic C used for component: C. Here, props is instead typed as Parameters<typeof mountComponent>[2], derived from mountComponent's own separate generic resolution rather than from this function's local C. When TypeScript extracts Parameters<> from a generic (non-overloaded) function type, the type parameter collapses to its constraint — so this resolves to ComponentProps<Component<any, any>>, regardless of which concrete component is passed to mountSettingView. In practice this means a caller could pass mismatched/incomplete props for the specific mounted component (e.g. ChoiceView, GlobalVariablesView) without a compile error, weakening exactly the kind of static safety this wrapper exists to preserve. Current call sites happen to pass correct props, so there's no active bug today, but the guard rail is gone for future changes.

♻️ Proposed fix

-	private mountSettingView<C extends Parameters<typeof mountComponent>[1]>(
-		setting: Setting,
-		component: C,
-		props: Parameters<typeof mountComponent>[2],
-	): MountHandle | null {
+	private mountSettingView<C extends Component<any, any>>(
+		setting: Setting,
+		component: C,
+		props: ComponentProps<C>,
+	): MountHandle | null {

(import Component/ComponentProps from svelte, mirroring mountComponent's own signature)

Since this hinges on how TypeScript's Parameters<> utility resolves a generic (non-overloaded) function's type parameters, please verify in the actual project TypeScript toolchain (e.g. by hovering props's resolved type or adding a deliberately-wrong prop at a call site) that this doesn't already silently accept mismatched props.

🤖 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/quickAddSettingsTab.ts` around lines 423 - 443, Update mountSettingView
so its props parameter is typed as ComponentProps<C>, coupling it to the
method’s existing component generic instead of using Parameters<typeof
mountComponent>[2]. Import ComponentProps from the same Svelte package used by
mountComponent, and verify that incorrect props at existing or temporary call
sites now produce TypeScript errors.

src/cli/registerQuickAddCliHandlers.test.ts (1)

36-54: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test helper reintroduces the exact unsafe cast this PR removes elsewhere.

walk((choice as IMultiChoice).choices!) bypasses the optional-choices contract with a non-null assertion, throwing at runtime if a Multi choice's choices is missing/non-array — the very malformed shape (#1566) this PR hardens against everywhere else via childChoicesOf(). Every other flattenChoices/traversal in this cohort (including the production one in this same file) was converted to use childChoicesOf; this local test helper is the one holdout.

♻️ Proposed fix

-			if (choice.type === "Multi") {
-				walk((choice as IMultiChoice).choices!);
-			}
+			if (choice.type === "Multi") {
+				walk(childChoicesOf(choice));
+			}

(and import childChoicesOf from ../utils/choiceUtils, dropping the now-unneeded IMultiChoice cast)

Based on learnings, IMultiChoice.choices is documented as optional/untrusted and meant to be read via childChoicesOf(), which this test helper bypasses.

🤖 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/cli/registerQuickAddCliHandlers.test.ts` around lines 36 - 54, Update the
local test helper flattenChoices and its walk traversal to use
childChoicesOf(choice) for Multi choices instead of casting to IMultiChoice and
asserting choices is non-null. Import childChoicesOf from ../utils/choiceUtils
and remove the now-unused IMultiChoice dependency, preserving traversal behavior
for valid child arrays while safely handling missing or malformed choices.

Source: Learnings

src/utils/packageTraversal.ts (1)

109-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the canonical choiceUtils helpers instead of reimplementing them inline. Both sites duplicate logic already centralized in choiceUtils.ts, undermining the PR's own stated goal of having one guard definition instead of many ad-hoc ones.

• src/utils/packageTraversal.ts#L109-L154: in buildChoiceCatalog's walk, replace isMultiChoice(choice) && Array.isArray(choice.choices) (Line 130) with hasChildChoices(choice), and pass childChoicesOf(choice) to the recursive walk call.
• src/gui/choiceList/contextMenu.ts#L50-L73: in computeEligibleMultiTargets, replace Array.isArray(roots) ? roots : [] (Line 55) with rootChoicesOf(roots), matching isChoiceNested's usage two functions above in the same file.

♻️ Proposed fixes

--- a/src/utils/packageTraversal.ts
@@
-			if (isMultiChoice(choice) && Array.isArray(choice.choices)) {
-				walk(choice.choices, choice.id, path);
-			}
+			if (hasChildChoices(choice)) {
+				walk(childChoicesOf(choice), choice.id, path);
+			}

--- a/src/gui/choiceList/contextMenu.ts
@@
-  const source: IChoice[] = Array.isArray(roots) ? roots : [];
+  const source: IChoice[] = rootChoicesOf(roots);

🤖 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/packageTraversal.ts` around lines 109 - 154, In
src/utils/packageTraversal.ts lines 109-154, update buildChoiceCatalog’s walk to
use hasChildChoices(choice) and recurse with childChoicesOf(choice) instead of
checking isMultiChoice and choice.choices directly. In
src/gui/choiceList/contextMenu.ts lines 50-73, update
computeEligibleMultiTargets to use rootChoicesOf(roots) instead of an inline
Array.isArray fallback, preserving the existing isChoiceNested helper usage.

🤖 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/gui/choiceList/ChoiceList.svelte`:
- Around line 50-57: Preserve malformed entries filtered by renderable when
applying drag or keyboard reorders: before calling onReorderChoices or assigning
the reordered choices, reintegrate the original list’s holes/non-choice entries
into the reordered choice positions using the existing hole-preservation helper
used elsewhere in the PR. Update both the drag items handling and moveChoice
path, and add a regression test confirming a null or stray primitive survives an
unrelated reorder.

---

Nitpick comments:
In `@src/cli/registerQuickAddCliHandlers.test.ts`:
- Around line 36-54: Update the local test helper flattenChoices and its walk
traversal to use childChoicesOf(choice) for Multi choices instead of casting to
IMultiChoice and asserting choices is non-null. Import childChoicesOf from
../utils/choiceUtils and remove the now-unused IMultiChoice dependency,
preserving traversal behavior for valid child arrays while safely handling
missing or malformed choices.

In `@src/migrations/helpers/isCaptureChoice.ts`:
- Around line 5-9: Update the isCaptureChoice parameter type from IChoice to
unknown, matching its null- and primitive-tolerant behavior and the sibling
isMultiChoice predicate. Keep the existing isChoiceLike and Capture type-guard
logic unchanged.

In `@src/quickAddSettingsTab.ts`:
- Around line 423-443: Update mountSettingView so its props parameter is typed
as ComponentProps<C>, coupling it to the method’s existing component generic
instead of using Parameters<typeof mountComponent>[2]. Import ComponentProps
from the same Svelte package used by mountComponent, and verify that incorrect
props at existing or temporary call sites now produce TypeScript errors.

In `@src/utils/packageTraversal.ts`:
- Around line 109-154: In src/utils/packageTraversal.ts lines 109-154, update
buildChoiceCatalog’s walk to use hasChildChoices(choice) and recurse with
childChoicesOf(choice) instead of checking isMultiChoice and choice.choices
directly. In src/gui/choiceList/contextMenu.ts lines 50-73, update
computeEligibleMultiTargets to use rootChoicesOf(roots) instead of an inline
Array.isArray fallback, preserving the existing isChoiceNested helper usage.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 675232b6-e5df-4a2a-8fc2-d313084d03a2

📥 Commits

Reviewing files that changed from the base of the PR and between bf39e2b and 0b5e722.

📒 Files selected for processing (40)

• src/choiceExecutor.ts
• src/cli/registerQuickAddCliHandlers.test.ts
• src/cli/registerQuickAddCliHandlers.ts
• src/gui/MacroGUIs/MacroBuilder.ts
• src/gui/PackageManager/ExportPackageModal.svelte
• src/gui/choiceList/ChoiceList.svelte
• src/gui/choiceList/ChoiceView.malformed.test.ts
• src/gui/choiceList/ChoiceView.svelte
• src/gui/choiceList/ChoicesUnavailable.svelte
• src/gui/choiceList/MultiChoiceListItem.svelte
• src/gui/choiceList/contextMenu.ts
• src/gui/suggesters/choiceSuggester.test.ts
• src/gui/suggesters/choiceSuggester.ts
• src/main.ts
• src/migrations/consolidateFileExistsBehavior.test.ts
• src/migrations/consolidateFileExistsBehavior.ts
• src/migrations/helpers/choice-traversal.ts
• src/migrations/helpers/isCaptureChoice.ts
• src/migrations/helpers/isMultiChoice.ts
• src/migrations/incrementFileNameSettingMoveToDefaultBehavior.ts
• src/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts
• src/migrations/removeMacroIndirection.test.ts
• src/migrations/removeMacroIndirection.ts
• src/quickAddSettingsTab.ts
• src/services/choiceService.audit-commands-choicelist.test.ts
• src/services/choiceService.insertIntoMulti.test.ts
• src/services/choiceService.malformed.test.ts
• src/services/choiceService.test.ts
• src/services/choiceService.ts
• src/services/packageExportService.test.ts
• src/services/packageImportService.malformed.test.ts
• src/services/packageImportService.test.ts
• src/services/packageImportService.ts
• src/types/choices/IMultiChoice.ts
• src/utils/choiceUtils.test.ts
• src/utils/choiceUtils.ts
• src/utils/malformedChoices.entrypoints.test.ts
• src/utils/malformedChoices.fixture.ts
• src/utils/packageTraversal.ts
• tests/packages/packageImportService.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 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/gui/choiceList/ChoiceView.malformed.test.ts`:
- Around line 129-142: Strengthen the test “drops entries that cannot be keyed
instead of losing the whole list” by asserting that both malformed entries—`{}`
and `{ name: "No id" }`—are absent from the rendered DOM, not only that one
keyed row remains. Keep the existing Survivor assertions and add checks against
the rendered output or relevant labels/text to verify neither malformed entry is
displayed.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 28e6d25b-f159-4063-8bea-5aa7cd0243d0

📥 Commits

Reviewing files that changed from the base of the PR and between 7f20581 and f06ef4e.

📒 Files selected for processing (2)

• src/gui/choiceList/ChoiceList.svelte
• src/gui/choiceList/ChoiceView.malformed.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/gui/choiceList/ChoiceList.svelte

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/utils/packageTraversal.ts (1)

109-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Same tree-walk-with-hole-guard logic triplicated across files.

The walk in buildChoiceCatalog (skip non-choice-like entries, recurse only into Multi with an array choices) is copied verbatim — including the same source comment — in src/services/choiceService.ts and src/gui/choiceList/contextMenu.ts. Since choiceUtils.ts already centralizes the safe-traversal contract (isChoiceLike, childChoicesOf, rootChoicesOf), consider extracting this walker into a single shared helper (e.g. a walkChoiceTree/buildChoiceIndex utility) so future hole-handling fixes only need to land once instead of being kept in sync across three call sites.

🤖 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/packageTraversal.ts` around lines 109 - 154, Extract the duplicated
choice-tree traversal from buildChoiceCatalog and its counterparts in
choiceService.ts and contextMenu.ts into a shared helper in choiceUtils.ts, such
as walkChoiceTree or buildChoiceIndex. Preserve the existing isChoiceLike hole
guard, rootChoicesOf/childChoicesOf traversal behavior, and recursion only for
multi-choice nodes with array choices; update all three call sites to use the
shared helper.

🤖 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/gui/choiceList/ChoiceView.malformed.test.ts`:
- Around line 95-99: Extend the malformed-choice regression test to also assert
that the nested-folder creation CTA is unavailable, alongside the existing
nested element and “Add a choice to Broken” assertions. Use the relevant
folder-creation label or query helper already used by the ChoiceView test, and
verify it resolves to null.

---

Nitpick comments:
In `@src/utils/packageTraversal.ts`:
- Around line 109-154: Extract the duplicated choice-tree traversal from
buildChoiceCatalog and its counterparts in choiceService.ts and contextMenu.ts
into a shared helper in choiceUtils.ts, such as walkChoiceTree or
buildChoiceIndex. Preserve the existing isChoiceLike hole guard,
rootChoicesOf/childChoicesOf traversal behavior, and recursion only for
multi-choice nodes with array choices; update all three call sites to use the
shared helper.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 060c7a08-3211-49cd-a127-65bfabdd60a7

📥 Commits

Reviewing files that changed from the base of the PR and between cb0599f and abb1b0c.

📒 Files selected for processing (40)

• src/choiceExecutor.ts
• src/cli/registerQuickAddCliHandlers.test.ts
• src/cli/registerQuickAddCliHandlers.ts
• src/gui/MacroGUIs/MacroBuilder.ts
• src/gui/PackageManager/ExportPackageModal.svelte
• src/gui/choiceList/ChoiceList.svelte
• src/gui/choiceList/ChoiceView.malformed.test.ts
• src/gui/choiceList/ChoiceView.svelte
• src/gui/choiceList/ChoicesUnavailable.svelte
• src/gui/choiceList/MultiChoiceListItem.svelte
• src/gui/choiceList/contextMenu.ts
• src/gui/suggesters/choiceSuggester.test.ts
• src/gui/suggesters/choiceSuggester.ts
• src/main.ts
• src/migrations/consolidateFileExistsBehavior.test.ts
• src/migrations/consolidateFileExistsBehavior.ts
• src/migrations/helpers/choice-traversal.ts
• src/migrations/helpers/isCaptureChoice.ts
• src/migrations/helpers/isMultiChoice.ts
• src/migrations/incrementFileNameSettingMoveToDefaultBehavior.ts
• src/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts
• src/migrations/removeMacroIndirection.test.ts
• src/migrations/removeMacroIndirection.ts
• src/quickAddSettingsTab.ts
• src/services/choiceService.audit-commands-choicelist.test.ts
• src/services/choiceService.insertIntoMulti.test.ts
• src/services/choiceService.malformed.test.ts
• src/services/choiceService.test.ts
• src/services/choiceService.ts
• src/services/packageExportService.test.ts
• src/services/packageImportService.malformed.test.ts
• src/services/packageImportService.test.ts
• src/services/packageImportService.ts
• src/types/choices/IMultiChoice.ts
• src/utils/choiceUtils.test.ts
• src/utils/choiceUtils.ts
• src/utils/malformedChoices.entrypoints.test.ts
• src/utils/malformedChoices.fixture.ts
• src/utils/packageTraversal.ts
• tests/packages/packageImportService.test.ts

🚧 Files skipped from review as they are similar to previous changes (33)

• src/cli/registerQuickAddCliHandlers.test.ts
• src/gui/MacroGUIs/MacroBuilder.ts
• src/services/packageExportService.test.ts
• src/migrations/helpers/isMultiChoice.ts
• src/gui/choiceList/ChoicesUnavailable.svelte
• src/migrations/removeMacroIndirection.test.ts
• src/gui/choiceList/MultiChoiceListItem.svelte
• src/types/choices/IMultiChoice.ts
• src/services/packageImportService.test.ts
• src/utils/malformedChoices.entrypoints.test.ts
• src/migrations/consolidateFileExistsBehavior.test.ts
• src/choiceExecutor.ts
• src/services/choiceService.insertIntoMulti.test.ts
• src/quickAddSettingsTab.ts
• src/gui/PackageManager/ExportPackageModal.svelte
• src/migrations/helpers/choice-traversal.ts
• src/cli/registerQuickAddCliHandlers.ts
• tests/packages/packageImportService.test.ts
• src/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts
• src/utils/malformedChoices.fixture.ts
• src/gui/choiceList/contextMenu.ts
• src/migrations/removeMacroIndirection.ts
• src/services/choiceService.malformed.test.ts
• src/utils/choiceUtils.ts
• src/gui/suggesters/choiceSuggester.ts
• src/main.ts
• src/gui/choiceList/ChoiceList.svelte
• src/migrations/incrementFileNameSettingMoveToDefaultBehavior.ts
• src/gui/suggesters/choiceSuggester.test.ts
• src/migrations/consolidateFileExistsBehavior.ts
• src/gui/choiceList/ChoiceView.svelte
• src/services/packageImportService.ts
• src/services/choiceService.ts