ENG-1490 Fix ZodError in getAllDiscourseNodes for legacy block prop shapes

[sid597]

https://www.loom.com/share/7d5857d8340e47fca362f3892c199d8c

Migrate legacy discourse node block props on read instead of dropping nodes that fail schema validation. Handles two known shape changes:

• specification: Condition[] → {enabled, query: {conditions, ...}}
• suggestiveRules.isFirstChild: {uid, value} → boolean

On successful migration, writes the fixed data back so subsequent reads hit the happy path. Nodes that can't be migrated still report to PostHog.

We're seeing recurring ZodErrors in PostHog from getAllDiscourseNodes. Two schema shape changes from the block props migration left user-created discourse nodes with stale
data that fails safeParse:

• specification changed from Condition[] to {enabled, query: {conditions, ...}}
• suggestiveRules.isFirstChild changed from {uid, value: boolean} to boolean

Default nodes get re-initialized on schema mismatch (via initSingleDiscourseNode), but user-created nodes were just silently dropped — they'd fail parse and never show up
in getAllDiscourseNodes.

Fix: When safeParse fails, we try migrating the known legacy shapes before dropping the node. If migration succeeds, we write the fixed data back to block props (so it's a
one-time cost) and include the node. If migration can't fix it, we still report to PostHog so we catch genuinely broken data.

What it doesn't do: No schema changes, no init restructuring. The migration is scoped to the two known shape changes and lives next to the read site where the error is
detected.

---

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of existing data structures. When initial data parsing encounters failures, the system now implements an automatic recovery mechanism that attempts to update and reprocess the data. If recovery succeeds, the updated data is properly stored and utilized; if unsuccessful, the original error handling is preserved, ensuring stability.

[linear]

ENG-1490 ZodError: DG Discourse Node Parse

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[sid597]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

A migration helper is introduced to transform legacy block property shapes during parsing. The getAllDiscourseNodes function now implements a retry mechanism: if initial parsing fails, it attempts to migrate block props and retry; if successful, it updates the props and returns migrated data, otherwise it preserves original error handling.

Changes

Cohort / File(s)

Summary

Block Props Migration apps/roam/src/components/settings/utils/accessors.ts

Added migrateNodeBlockProps helper to transform legacy block prop shapes (Specification and suggestiveRules). Modified getAllDiscourseNodes to implement parse-and-migrate retry logic: on initial parse failure, attempts migration and retries; on success, updates block props via setBlockProps; on failure, preserves original error path. Updated node text derivation to use prefix-stripped title instead of inline replacement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• ENG-1291: Port discourse node specification #765: Introduces migration logic for legacy block prop shapes and schema changes that directly align with the new migrateNodeBlockProps transformations.
• ENG-1468: Port node and relations tree based consumers #825: Modifies the same apps/roam/src/components/settings/utils/accessors.ts file to consolidate setting accessor functions.
• ENG-1187: Prod: Implement get and set for block prop based settings #669: Introduces the original getAllDiscourseNodes functionality that this PR extends with migration and retry logic.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly reflects the main change: fixing a ZodError in getAllDiscourseNodes for legacy block prop shapes, which aligns with the core objective of migrating legacy data structures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

---

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

[coderabbitai]

🧹 Nitpick comments (1)

apps/roam/src/components/settings/utils/accessors.ts (1)

594-601: Consider reporting retryResult.error instead of result.error.

When migration is attempted but still fails validation, the retryResult.error would be more informative for debugging since it reflects the error against the migrated data structure. The original result.error shows why the unmigrated props failed, which is expected.

💡 Suggested change

       } else {
         internalError({
-          error: result.error,
+          error: retryResult.error,
           type: "DG Discourse Node Parse",
           context: { pageUid, title },
           sendEmail: false,
         });
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/roam/src/components/settings/utils/accessors.ts` around lines 594 - 601,
The internalError call currently logs result.error (the original validation
error) after a migration retry; instead use retryResult.error so the logged
error reflects validation against the migrated data. Update the error argument
passed to internalError in the else branch (the block that currently references
result.error) to retryResult.error while keeping the same type ("DG Discourse
Node Parse") and context ({ pageUid, title }) and sendEmail flag.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/roam/src/components/settings/utils/accessors.ts`:
- Around line 594-601: The internalError call currently logs result.error (the
original validation error) after a migration retry; instead use
retryResult.error so the logged error reflects validation against the migrated
data. Update the error argument passed to internalError in the else branch (the
block that currently references result.error) to retryResult.error while keeping
the same type ("DG Discourse Node Parse") and context ({ pageUid, title }) and
sendEmail flag.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5546d478-8daa-40b8-9b06-38f34ec035db

📥 Commits

Reviewing files that changed from the base of the PR and between cd30dd5 and d441245.

📒 Files selected for processing (1)

• apps/roam/src/components/settings/utils/accessors.ts

[mdroidian]

Great video! Very clear, to the point. 💥