chore: fix race conditions, hydration, build issues#616
chore: fix race conditions, hydration, build issues#616merivercap wants to merge 1 commit intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAdds retry logic and race-condition mitigation for thread-related DB operations, adjusts signup preference creation to match Hasura API, downgrades markdown/math packages and wiring to fix hydration issues, updates Next/TS config and paths for workspace packages, and tweaks tests, seeds, and markdown usage to align with the new behavior and typings. Class diagram for JSX type patch and MemoizedReactMarkdown usageclassDiagram
class ReactJSXIntrinsicElements {
}
class JSXIntrinsicElements {
}
class JSXNamespace {
}
class MemoizedReactMarkdown {
+Options props
+render()
}
class RemarkMath {
}
class RehypeKatex {
}
class ReactMarkdownCore {
}
JSXNamespace <|-- JSXIntrinsicElements : contains
JSXIntrinsicElements ..|> ReactJSXIntrinsicElements : extends
MemoizedReactMarkdown --> ReactMarkdownCore : wraps
MemoizedReactMarkdown --> RemarkMath : remarkPlugins
MemoizedReactMarkdown --> RehypeKatex : rehypePlugins
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
getPromptDetailsthecatchblock logserrorbut the caught variable is namederr, so the console.error is currently referencing an undefined identifier; switch to loggingerr(or rename the parameter) to avoid masking real failures. - The
insertPreferencesByUserIdcall in the signup route still has a multi-line inline comment about its signature and omits thejwtargument it mentions; consider either updating the comment to reflect the current API or passing the required parameters so the implementation and commentary stay in sync. - You’ve added
seed.logandseed_error.logto the repo; if these are runtime artefacts rather than canonical seed inputs, it may be better to remove them from version control and add them to.gitignoreto avoid noisy diffs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `getPromptDetails` the `catch` block logs `error` but the caught variable is named `err`, so the console.error is currently referencing an undefined identifier; switch to logging `err` (or rename the parameter) to avoid masking real failures.
- The `insertPreferencesByUserId` call in the signup route still has a multi-line inline comment about its signature and omits the `jwt` argument it mentions; consider either updating the comment to reflect the current API or passing the required parameters so the implementation and commentary stay in sync.
- You’ve added `seed.log` and `seed_error.log` to the repo; if these are runtime artefacts rather than canonical seed inputs, it may be better to remove them from version control and add them to `.gitignore` to avoid noisy diffs.
## Individual Comments
### Comment 1
<location> `apps/pro-web/app/actions/subscriptions.actions.ts:205-206` </location>
<code_context>
+ }[]
+ ).map((input) => [input.label, '']),
)
} catch (err) {
console.error('Error fetching prompt details:', error)
</code_context>
<issue_to_address>
**issue (bug_risk):** The catch block logs an undefined `error` variable instead of the caught `err`.
In this catch block, `err` is the caught exception, but `console.error` references `error`, which is undefined in this scope and will throw a `ReferenceError`, masking the real issue. Use `err` in the log call instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } catch (err) { | ||
| console.error('Error fetching prompt details:', error) |
There was a problem hiding this comment.
issue (bug_risk): The catch block logs an undefined error variable instead of the caught err.
In this catch block, err is the caught exception, but console.error references error, which is undefined in this scope and will throw a ReferenceError, masking the real issue. Use err in the log call instead.
📝 WalkthroughWalkthroughThis PR introduces a comprehensive nested accordion-based document thread outline UI system with associated data models and rendering pipelines, adds retry-based resilience to async operations, downgrades markdown-related dependencies, establishes new TypeScript path aliases, and applies bug fixes for undefined value handling across markdown rendering components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/pro-web/app/api/auth/signup/route.ts (2)
129-142:⚠️ Potential issue | 🟡 MinorRemove developer thinking notes from production code.
Lines 130–134 contain internal reasoning comments ("Wait, let me check…") that read like debug notes rather than documentation. They should be cleaned up before merging.
Proposed fix
const { data: preference, error } = await insertPreferencesByUserId({ - // userId is not needed in the object for this specific function signature in hasura.service, - // check the definition of insertPreferencesByUserId manually? - // Wait, let me check the signature of insertPreferencesByUserId again. - // It takes { jwt, preferencesSet }. preferencesSet is PreferenceInsertInput. - // PreferenceInsertInput has userId. preferencesSet: { userId: insertUserOne.userId, preferredComplexity: 'general', preferredLength: 'detailed', preferredTone: 'neutral', preferredType: 'mix', }, })
129-142:⚠️ Potential issue | 🔴 CriticalFix the pro-web
insertPreferencesByUserIdmutation return shape and use valid enum values.Two confirmed issues prevent user signup in pro-web:
Invalid mutation return shape: The function selects
preferences: { returning: { __scalar: true } }, but thePreferencereturn type has nopreferencesfield. It should select individual fields like the web version does:lang: true, fontSize: true, preferenceId: true. The current selection will fail with a GraphQL query error.Invalid enum values: The defaults use
'general'and'mix', but the database enum tables only contain:
complexity_enum:expert,adult,eli5(not'general')type_enum:narrative,step_by_step,bullet_points,neutral(not'mix')These FK constraint violations will cause the insert to fail at the database level. Use valid enum values:
preferredComplexity: 'eli5'(or'expert'/'adult') andpreferredType: 'narrative'(or'step_by_step'/'bullet_points'/'neutral').
🤖 Fix all issues with AI agents
In `@apps/pro-web/lib/select-scope-conversation-window.test.ts`:
- Around line 297-306: Update the ScopeConversationWindow type so lastUser and
lastAssistant are typed as Message (not Message | UIMessage): replace the union
in the declaration of ScopeConversationWindow (fields lastUser, lastAssistant)
to use Message directly and adjust any imports to remove UIMessage if no longer
used; ensure userQuestions remains Array<Message> so callers/tests can access
messageId without casting.
In `@apps/pro-web/package.json`:
- Around line 94-101: Update the markdown deps and reconcile math renderers:
change remark-math from 3.0.1 to 5.1.1 in apps/pro-web/package.json to remove
the type incompatibility that required the `@ts-expect-error` in
apps/pro-web/components/shared/markdown.tsx (look for the MemoizedReactMarkdown
usage), then run install and remove the `@ts-expect-error` in that file if types
are satisfied; also decide whether to standardize on rehype-katex or
rehype-mathjax by either removing the unused rehype-mathjax@4.0.0 dependency or
integrating it into the main MemoizedReactMarkdown pipeline (replace
rehype-katex usage or add rehype-mathjax to the same plugins list) so math
rendering is consistent across chat components and the main markdown renderer.
In `@Phase_0_Chat_Refactor.md`:
- Around line 7-15: The document contains broken citation link fragments like
[0-cite-N](`#0-cite-N`) across multiple sections (e.g., in definitions for
OutlineNode, MarkdownSection, MessagePair, Scope/ScopeId, and Level vs Depth);
fix by either adding matching anchor targets (e.g., create corresponding
headings or anchor IDs named 0-cite-0, 0-cite-1, etc.) or replacing each inline
link with a plain-text citation (e.g., “see citation 0-cite-0”) for the
referenced concepts (OutlineNode, MarkdownSection, MessagePair, Scope/ScopeId,
Level vs Depth) so the references resolve correctly.
🧹 Nitpick comments (9)
apps/pro-web/lib/select-scope-conversation-window.test.ts (1)
207-208:null as unknown as stringdouble-cast is acceptable for edge-case testing, but consider a helper.The triple repetition of
null as unknown as stringcould be extracted into a small constant (e.g.,const NULL_SCOPE = null as unknown as string) at the top of the describe block, next toROOTandSECTION, to reduce noise and make intent clearer.♻️ Suggested constant extraction
const ROOT = 'doc-root-1' const SECTION = 'heading-1' +const NULL_SCOPE = null as unknown as stringThen replace all three occurrences:
- scopeId: null as unknown as string, + scopeId: NULL_SCOPE,Also applies to: 223-224, 274-274
apps/pro-web/app/b/page.tsx (1)
4-6: Unusedpropsparameter.
props(typed asPageProps) is declared but never referenced in the function body. If the page doesn't need route params/searchParams, consider removing it to avoid dead code.Suggested fix
-export default async function BotPage(props: PageProps) { +export default async function BotPage() { // When no bot is selected, pass null/undefined to show welcome view return <BotProfileThreadSection threads={[]} count={0} chatbot={null} /> }apps/pro-web/components/routes/chat/chat-selected-chatbot-mobile.tsx (1)
94-94: Redundant default export alongside named export.
SelectedBotMobileViewis already a named export on line 15. The default export on line 94 is unnecessary and goes against the project's preference for named exports. As per coding guidelines: "Avoid default exports; prefer named exports to make imports clearer and help avoid naming conflicts."Suggested fix
} - -export default SelectedBotMobileViewapps/pro-web/app/actions/thread.actions.ts (1)
333-350: Retry loop is reasonable but total worst-case latency (~15s) is worth noting.The exponential backoff (1s → 2s → 4s → 8s) totals ~15 seconds before the final attempt. For a server action, this could approach timeout thresholds or noticeably degrade UX. Consider whether fewer retries or a shorter base delay would suffice, or whether the root cause (thread not yet persisted when this runs) can be addressed upstream with a more deterministic signal.
Also, the
console.warnonly logs intermediate retry attempts. Adding a final warning before the throw on Line 350 would help with debugging production failures.Suggested improvement
} - if (!existingThread) throw new Error('Thread not found for slug') + if (!existingThread) { + console.error( + `[uploadWorkspaceDocumentToBucket] Thread not found for slug ${threadSlug} after ${MAX_RETRIES} attempts.`, + ) + throw new Error('Thread not found for slug') + }apps/pro-web/components/shared/image-message.tsx (1)
31-35: Caller-supplied plugins overrideMemoizedReactMarkdown's built-in plugins.
MemoizedReactMarkdownnow hardcodesremarkMath+rehypeKatexinternally (inmarkdown.tsx), but the{...props}spread overwrites them. This means this component getsremarkGfm+rehypeMathJaxwhileMemoizedReactMarkdownelsewhere usesremarkGfm+remarkMath+rehypeKatex— two different math rendering stacks (MathJax vs KaTeX).If this is intentional, consider adding a brief comment explaining why this component uses a different math pipeline. If not, consider dropping the explicit plugin props here and relying on the shared defaults.
apps/pro-web/components/shared/markdown.tsx (1)
10-18:transformLatexBlocksconverts inline math\(...\)to display math$$...$$.Since
singleDollarTextMath: falsedisables single-$parsing, there's no way to produce inline math — both\[...\]and\(...\)end up as$$...$$(display/block). If inline math rendering matters, consider mapping\(...\)to$...$and enablingsingleDollarTextMath, or using a different delimiter strategy. This appears to be pre-existing behavior, so it can be deferred.apps/pro-web/app/actions/chat-memory/ingest-assistant-answer.ts (1)
64-70: Pre-existing race:nextTurnIndexcomputed outside the retry loop.If multiple concurrent ingest calls target the same
threadIdwhile the thread doesn't yet exist, they'll all computenextTurnIndex = 1(sincefindFirstreturns nothing). Once the thread appears and retries succeed, all inserts land withturnIndex = 1. There's no unique constraint on(threadId, turnIndex), so the DB accepts them, but the logical ordering is lost.This isn't introduced by this PR, but if turn ordering matters, consider either:
- Moving the
nextTurnIndexquery inside the retry loop, or- Using a DB sequence /
SELECT MAX(...) + 1inside a transaction.apps/pro-web/app/api/auth/signup/route.ts (1)
2-9: Unused import:sendEmailVerification.
sendEmailVerification(line 3) is imported but never called — the verification flow at lines 173-190 generates a token but returns before sending an email. This appears to be pre-existing dead code, but worth noting for cleanup.Phase_0_Chat_Refactor.md (1)
1-1: File name doesn't follow the repository's naming convention.The coding guidelines specify lowercase with dash separators for file names.
Phase_0_Chat_Refactor.mdshould be renamed tophase-0-chat-refactor.md. As per coding guidelines: "Use lowercase with dash separators for directory and file names (e.g.,components/auth-wizard)".
| expect(res.userQuestions.map((m) => (m as any).messageId)).toEqual([ | ||
| 'u1', | ||
| 'u2', | ||
| ]) | ||
|
|
||
| // lastAssistant: latest assistant message | ||
| expect(res.lastAssistant?.messageId).toBe('a1') | ||
| expect((res.lastAssistant as any)?.messageId).toBe('a1') | ||
|
|
||
| // lastUser: latest user message even if excluded from q-chain | ||
| expect(res.lastUser?.messageId).toBe('u3') | ||
| expect((res.lastUser as any)?.messageId).toBe('u3') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "select-scope-conversation-window" --type fRepository: bitcashorg/masterbots
Length of output: 122
🏁 Script executed:
fd "select-scope-conversation-window" --type f | grep -v testRepository: bitcashorg/masterbots
Length of output: 47
🏁 Script executed:
# Search for the function definition in the codebase
rg "selectScopeConversationWindow" --type ts --type tsx -lRepository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
rg "selectScopeConversationWindow" --type ts -lRepository: bitcashorg/masterbots
Length of output: 456
🏁 Script executed:
# Also search for the function definition without type restrictions
rg "function selectScopeConversationWindow|const selectScopeConversationWindow|export.*selectScopeConversationWindow"Repository: bitcashorg/masterbots
Length of output: 194
🏁 Script executed:
cat -n apps/pro-web/app/actions/chat-memory/get-scope-conversation-window.tsRepository: bitcashorg/masterbots
Length of output: 6862
🏁 Script executed:
rg "type Message" apps/pro-web --type ts -A 10 | head -50Repository: bitcashorg/masterbots
Length of output: 3218
🏁 Script executed:
# Look for mb-genql types
find apps/pro-web -name "*mb-genql*" -o -name "*generated*" -type f | head -20Repository: bitcashorg/masterbots
Length of output: 115
🏁 Script executed:
# Check the Message type structure in the codebase
rg "messageId" apps/pro-web/components/routes/browse/browse-chat-messages.tsx -B 2 -A 2Repository: bitcashorg/masterbots
Length of output: 209
🏁 Script executed:
# Look at the test file to understand the exact issue
sed -n '297,306p' apps/pro-web/lib/select-scope-conversation-window.test.tsRepository: bitcashorg/masterbots
Length of output: 383
🏁 Script executed:
# Check the full test context to understand what the function should return
sed -n '280,310p' apps/pro-web/lib/select-scope-conversation-window.test.tsRepository: bitcashorg/masterbots
Length of output: 893
🏁 Script executed:
# Check what other uses of selectScopeConversationWindow expect
rg "selectScopeConversationWindow" apps/pro-web -A 5 -B 2 | grep -v "test.ts" | head -40Repository: bitcashorg/masterbots
Length of output: 2120
🏁 Script executed:
# Check how the window result is used in use-workspace-chat.tsx
rg "window\." apps/pro-web/lib/hooks/use-workspace-chat.tsx -B 2 -A 2 | head -50Repository: bitcashorg/masterbots
Length of output: 503
🏁 Script executed:
# Check how it's used in chat-context-utils.ts
rg "selectScopeConversationWindow" apps/pro-web/lib/chat/chat-context-utils.ts -A 15 | head -40Repository: bitcashorg/masterbots
Length of output: 1167
🏁 Script executed:
# Check the Message type definition from mb-genql more closely
rg "from 'mb-genql'" apps/pro-web/services/hasura/hasura.service.ts -B 5 -A 20 | head -60Repository: bitcashorg/masterbots
Length of output: 539
🏁 Script executed:
# Check if there's a type definition file for Message
find apps/pro-web -name "*.type.ts" -o -name "types.ts" | xargs grep -l "Message" 2>/dev/null | head -5Repository: bitcashorg/masterbots
Length of output: 116
🏁 Script executed:
# Check the message type definition in the service type file
cat apps/pro-web/services/hasura/hasura.service.type.ts | head -100Repository: bitcashorg/masterbots
Length of output: 2321
🏁 Script executed:
# Also check what Message looks like when imported from mb-genql
rg "type Message|interface Message" --type ts -B 2 -A 10 | grep -A 10 "mb-genql" | head -30Repository: bitcashorg/masterbots
Length of output: 2034
🏁 Script executed:
# Get the full Message interface definition
rg "export interface Message \{" packages/mb-genql/generated/schema.ts -A 30 | head -40Repository: bitcashorg/masterbots
Length of output: 1208
🏁 Script executed:
# Check what UIMessage looks like from `@ai-sdk/react`
rg "import.*UIMessage" apps/pro-web --type ts -B 2 -A 5 | head -20Repository: bitcashorg/masterbots
Length of output: 1704
🏁 Script executed:
# Check the actual return type used in practice - look at the cast in selectScopeConversationWindow
sed -n '170,185p' apps/pro-web/app/actions/chat-memory/get-scope-conversation-window.tsRepository: bitcashorg/masterbots
Length of output: 538
Update return type to exclude UIMessage union—the function always returns Message which includes messageId.
The ScopeConversationWindow type declares lastUser and lastAssistant as Message | UIMessage, but the function always returns Message instances (see lines 177–179). Since UIMessage from the ai package lacks messageId, the union forces callers and tests to cast to Message or any to access messageId. This is unnecessary and creates a false type safety gap.
The query explicitly selects messageId, and the Message type from mb-genql declares it as a required field. Change the ScopeConversationWindow type definition to use Message directly instead of the union:
export type ScopeConversationWindow = {
lastUser?: Message
lastAssistant?: Message
userQuestions: Array<Message>
}This eliminates the type-safety workaround and makes the actual return contract explicit.
🤖 Prompt for AI Agents
In `@apps/pro-web/lib/select-scope-conversation-window.test.ts` around lines 297 -
306, Update the ScopeConversationWindow type so lastUser and lastAssistant are
typed as Message (not Message | UIMessage): replace the union in the declaration
of ScopeConversationWindow (fields lastUser, lastAssistant) to use Message
directly and adjust any imports to remove UIMessage if no longer used; ensure
userQuestions remains Array<Message> so callers/tests can access messageId
without casting.
| "react-markdown": "8.0.7", | ||
| "react-syntax-highlighter": "^15.6.1", | ||
| "react-textarea-autosize": "^8.5.7", | ||
| "react-use": "^17.6.0", | ||
| "rehype-katex": "^7.0.1", | ||
| "rehype-mathjax": "^7.1.0", | ||
| "remark-gfm": "4.0.1", | ||
| "remark-math": "^6.0.0", | ||
| "rehype-katex": "6.0.3", | ||
| "rehype-mathjax": "4.0.0", | ||
| "remark-gfm": "3.0.1", | ||
| "remark-math": "3.0.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check how remark-math and related packages are used
rg -n "remark-math|rehype-mathjax|rehype-katex|math" --type ts --type tsx -B2 -A2 2>/dev/null | head -100
# Check if there are any version compatibility notes in comments or docs
fd "README|CHANGELOG|NOTES" --type f | xargs grep -l "remark-math\|rehype-mathjax" 2>/dev/null || echo "No compatibility notes found"
# Check package-lock or lock files for dependency resolution info
fd "package-lock.json|pnpm-lock.yaml|yarn.lock|bun.lockb" --type f 2>/dev/null | head -5Repository: bitcashorg/masterbots
Length of output: 93
🏁 Script executed:
# Check the current package.json to see if there are any comments or notes about the downgrades
sed -n '90,110p' apps/pro-web/package.jsonRepository: bitcashorg/masterbots
Length of output: 637
🏁 Script executed:
# Look for where these plugins are configured in the code
rg -n "usePlugins|Plugin|remark|rehype" --type ts --type tsx -g "*/markdown*" -g "*/react-markdown*" -B2 -A2 2>/dev/null | head -80Repository: bitcashorg/masterbots
Length of output: 47
🏁 Script executed:
# Check if there's a components using react-markdown with the plugins
fd -e tsx -e ts | xargs grep -l "react-markdown" 2>/dev/null | head -10Repository: bitcashorg/masterbots
Length of output: 289
🏁 Script executed:
# Check the markdown component implementations
cat -n apps/pro-web/components/shared/markdown.tsxRepository: bitcashorg/masterbots
Length of output: 1707
🏁 Script executed:
# Check the memoized markdown components
cat -n apps/pro-web/lib/memoized-markdown-components.tsxRepository: bitcashorg/masterbots
Length of output: 7573
🏁 Script executed:
# Check the react-markdown patch - this is important!
cat -n apps/pro-web/types/react-markdown-patch.d.tsRepository: bitcashorg/masterbots
Length of output: 421
🏁 Script executed:
# Check if rehype-mathjax is actually used in the codebase
rg -n "rehype-mathjax|rehypeMathJax" --type ts --type tsxRepository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
# Verify the exact versions currently in package.json
grep -A2 -B2 "rehype-katex\|rehype-mathjax\|remark-math" apps/pro-web/package.jsonRepository: bitcashorg/masterbots
Length of output: 287
🏁 Script executed:
# Check if rehype-mathjax is used anywhere
rg -n "rehype-mathjax|mathJax" -g "*.ts" -g "*.tsx"Repository: bitcashorg/masterbots
Length of output: 350
🏁 Script executed:
# Also check JavaScript files
rg -n "rehype-mathjax" --type jsRepository: bitcashorg/masterbots
Length of output: 47
🏁 Script executed:
# Check the entire apps/pro-web directory for any rehype-mathjax usage
find apps/pro-web -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) -exec grep -l "rehype-mathjax" {} \;Repository: bitcashorg/masterbots
Length of output: 169
🏁 Script executed:
# Check how rehype-mathjax is used in chat-message-pro.tsx
cat -n apps/pro-web/components/routes/pro/chat-message-pro.tsx | head -50Repository: bitcashorg/masterbots
Length of output: 2420
🏁 Script executed:
# Check how rehype-mathjax is used in image-message.tsx
cat -n apps/pro-web/components/shared/image-message.tsx | head -50Repository: bitcashorg/masterbots
Length of output: 1949
🏁 Script executed:
# Search for where rehypeMathJax is actually passed to plugins
rg -n "rehypeMathJax|rehype-mathjax" -B5 -A5 -g "*.ts" -g "*.tsx"Repository: bitcashorg/masterbots
Length of output: 6367
Remark-math v3.0.1 causes documented type incompatibilities — use v5.1.1 as validated in prior work.
The markdown component has @ts-expect-error - remark-math 3 types incompatible with react-markdown 8 (line 26 in apps/pro-web/components/shared/markdown.tsx), confirming the type issues with v3.0.1. Prior learning documented that v5.1.1 was the validated stable downgrade, and the current further downgrade to v3.0.1 introduces suppressions that mask type safety.
Additionally, rehype-mathjax@4.0.0 is imported in chat components but not integrated into the main MemoizedReactMarkdown pipeline, which uses rehype-katex instead. This creates inconsistency in math rendering engines.
Update remark-math to 5.1.1 and verify whether rehype-mathjax should be removed or properly integrated with the main markdown rendering stack.
🤖 Prompt for AI Agents
In `@apps/pro-web/package.json` around lines 94 - 101, Update the markdown deps
and reconcile math renderers: change remark-math from 3.0.1 to 5.1.1 in
apps/pro-web/package.json to remove the type incompatibility that required the
`@ts-expect-error` in apps/pro-web/components/shared/markdown.tsx (look for the
MemoizedReactMarkdown usage), then run install and remove the `@ts-expect-error`
in that file if types are satisfied; also decide whether to standardize on
rehype-katex or rehype-mathjax by either removing the unused
rehype-mathjax@4.0.0 dependency or integrating it into the main
MemoizedReactMarkdown pipeline (replace rehype-katex usage or add rehype-mathjax
to the same plugins list) so math rendering is consistent across chat components
and the main markdown renderer.
There was a problem hiding this comment.
Hm, this is a big version revert... Yes, we have to definitely test these plugins and markdown. Markdown wasn't having issues to begging with though, since Markdown the only issue that we had is that we made a major markdown dependency update and that caused some issues, however, was stabilised long ago and only a few of maths formulas are difficult to take.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check git history for package.json to see what markdown versions were used before
cd apps/pro-web
git log --oneline --all -20 -- package.json | head -20
Length of output: 918
🏁 Script executed:
#!/bin/bash
# Check what remark-math version was used before recent changes
cd apps/pro-web
git log -p --all -S "remark-math" -- package.json | grep -A2 -B2 "remark-math" | head -40
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for any mentions of version 5.1.1 in the git history
cd apps/pro-web
git log --all --grep="5.1.1" --oneline | head -10
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find the commit where react-markdown was at version 8.0.7 previously
cd apps/pro-web
git log -p --all -- package.json | grep -B5 -A5 '"react-markdown": "8.0.7"' | head -30
Length of output: 0
@coderabbit-user Thanks — agreed, this is a big revert and we should validate it end-to-end. Quick status from the codebase today:
- We’re reverting react-markdown to 8.0.7 and remark-gfm to 3.0.1, but remark-math is currently set to 3.0.1. Our prior stable combo recorded in our learnings was react-markdown 8.0.7 + remark-gfm 3.0.1 + remark-math 5.1.1, which avoided the type incompatibilities that forced us to add ts-expect-error in apps/pro-web/components/shared/markdown.tsx.
- We’re mixing math engines: shared Markdown uses rehype-katex, while chat-message-pro.tsx and image-message.tsx wire up rehype-mathjax. That split is likely why “a few formulas are difficult” and can also affect hydration/SSR consistency.
Minimal, safe follow-ups I recommend:
- Set remark-math back to 5.1.1 (the previously validated version) to remove the type suppressions and align with the known-good stack.
- Standardize on one math renderer by default:
- Default: KaTeX in the shared Markdown pipeline for speed and SSR/hydration stability.
- Optional: Allow MathJax only in pro chat views behind a prop/flag when we detect unsupported KaTeX syntax, so advanced edge cases still render.
Proposed validation checklist (fast to run locally/CI):
- Build + type check: verify the ts-expect-error lines in apps/pro-web/components/shared/markdown.tsx can be removed once remark-math is 5.1.1.
- Hydration sanity: open ChatMessagePro and any page using the shared Markdown component, confirm no hydration warnings in the console.
- Rendering samples:
- GFM tables/lists/checkboxes render uniformly.
- KaTeX coverage: inline
$...$ , block$$...$$ , fractions, aligned equations (aligned with our typical docs). - One or two “hard” formulas (known problematic ones) in ChatMessagePro with MathJax flag on, to confirm the fallback works.
If you’re good with this direction, I can prepare a tiny follow-up: bump remark-math to 5.1.1, keep react-markdown at 8.0.7, unify the default renderer to KaTeX in the shared Markdown, and gate MathJax use in the chat components via a prop so we maintain consistency while preserving advanced coverage.
Also noting I used our previous learning about the stable versions to inform this suggestion.
✏️ Learnings added
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 616
File: apps/pro-web/package.json:94-101
Timestamp: 2026-02-24T17:16:43.775Z
Learning: In apps/pro-web, the previously stable markdown stack is react-markdown 8.0.7, remark-gfm 3.0.1, and remark-math 5.1.1; default renderer should be KaTeX for SSR/hydration, with MathJax only when advanced syntax is required in chat views.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2025-01-30T21:56:31.907Z
Learning: The react-markdown package version ^9.0.3 and its related dependencies (remark-gfm ^4.0.0, remark-math ^6.0.0) caused component crashes in the markdown rendering components. The packages were downgraded to react-markdown ^8.0.7, remark-gfm ^3.0.1, and remark-math ^5.1.1 to fix the issues.
Learnt from: Bran18
Repo: bitcashorg/masterbots PR: 337
File: package.json:32-34
Timestamp: 2025-01-21T16:52:47.056Z
Learning: The markdown rendering in this codebase uses `react-markdown` with `remark-gfm` and `remark-math` plugins for standard markdown and GitHub Flavored Markdown support. Raw HTML parsing through `rehype-raw` is not needed.
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T18:53:44.700Z
Learning: Applies to apps/pro-web/**/*.{ts,tsx} : Use dedicated utility functions for Markdown processing in Pro Workspace
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-06T18:53:00.520Z
Learning: Applies to **/pro-web/**/*.{jsx,tsx} : For Pro Workspace editor components, follow controlled/uncontrolled patterns, use dedicated utility functions for Markdown processing, and manage section state synchronization carefully
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-06T18:52:26.701Z
Learning: Applies to apps/pro-web/components/**/*.{tsx,jsx} : For Pro Workspace, editor components follow controlled/uncontrolled patterns, use dedicated utility functions for Markdown processing, manage section state carefully, and consider timing for auto-scroll and DOM manipulation
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2024-12-05T01:08:50.085Z
Learning: The project is a monorepo using Turborepo with Next.js frontend (apps/masterbots.ai), Hasura backend (apps/hasura), and shared packages for types, GraphQL operations, and utilities.
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-06T18:52:26.701Z
Learning: Applies to **/*.test.{ts,tsx} : Use Jest for testing framework and React Testing Library for component testing when testing is required
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-06T18:52:26.701Z
Learning: Applies to **/*.{tsx,jsx} : Use Tailwind CSS utility classes for styling React components for faster development, consistent design, responsive design, maintainability, and small CSS footprint
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T18:53:44.700Z
Learning: Applies to apps/pro-web/**/*.{tsx,jsx} : DOM manipulation and auto-scroll in Pro Workspace require timing considerations with requestAnimationFrame or delays
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T18:53:44.700Z
Learning: Applies to **/*.{tsx,jsx} : Order React component files: Imports → Constants → Component → Styled Components → Types/Interfaces
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2025-09-04T21:31:18.830Z
Learning: The pro-web workspace system has identified performance bottlenecks in markdown parsing, AI streaming response handling, selection range management, Base64 document processing, and multiple debounced state operations that require optimization and better coordination.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2026-01-22T23:10:37.056Z
Learning: AndlerRL emphasizes Modular Development: avoid duplication by centralizing shared utilities/types, prefer small composable units, and keep flows simple and explicit (no cumbersome validations or over-engineering).
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 542
File: apps/web/components/layout/profile/layout.tsx:67-68
Timestamp: 2025-08-29T20:34:24.433Z
Learning: AndlerRL prefers skeleton loading states over conditional rendering for better UX when components are waiting for data to load. Components should show loading indicators rather than blank spaces.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2025-11-07T18:41:56.562Z
Learning: Successfully created comprehensive GitHub issue `#581` consolidating 16 unresolved observations from PR `#579` review, including critical TDZ crash, migration bugs, transaction safety issues, and validation improvements, with specific file paths, line numbers, and action items organized by priority (4 critical, 8 major, 2 minor, 2 acknowledged for future PRs).
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2026-02-03T19:09:24.567Z
Learning: bitcashorg/masterbots: For RAG-related refactors, AndlerRL prefers a single consolidated tracking issue that links all relevant PR comments and issues and organizes observations by priority/status (P0/P1/P2), to accompany the active PR.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2025-09-04T21:31:18.830Z
Learning: Successfully created comprehensive GitHub issue `#555` consolidating performance feedback and edge-case management for pro-web workspace features, including prioritized action items, testing requirements, and specific file references for implementation.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2025-11-21T19:47:50.195Z
Learning: For repo bitcashorg/masterbots (apps/pro-web), AndlerRL prefers integrating TanStack Query into React Context Hooks wherever they fit to manage shared server state and side effects, with a longer-term plan to align broader state management with TanStack Query and potentially replace React Context hooks. Captured from PR `#584` discussion on Nov 21, 2025.
| **OutlineNode** - Tree data structure representing markdown document hierarchy with stable IDs, nesting depth, and parent-child relationships. [0-cite-0](#0-cite-0) | ||
|
|
||
| **MarkdownSection** - Raw parsed markdown heading with embedded IDs, content boundaries, and absolute offset tracking for surgical edits. [0-cite-1](#0-cite-1) | ||
|
|
||
| **MessagePair** - User-assistant message grouping unit that pairs questions with responses for accordion rendering. [0-cite-2](#0-cite-2) | ||
|
|
||
| **Scope/ScopeId** - Section identifier (`section.id` or `node.id`) used to group messages by document sections, with special handling for root (`__doc_root__`) and orphaned messages (`__orphaned__::<scopeId>`). [0-cite-3](#0-cite-3) | ||
|
|
||
| **Level vs Depth** - `level` refers to markdown heading level (1-6), while `depth` refers to tree nesting depth (0+ from root). Critical distinction for styling logic. [0-cite-4](#0-cite-4) |
There was a problem hiding this comment.
All citation link fragments are broken.
Every [0-cite-N](#0-cite-N) reference resolves to a non-existent anchor. These appear throughout the entire document (lines 7, 9, 11, 13, 15, 25, 27, 29, 46, 48, 50, 54, 56, 66, 70, 72, etc.). Either define the anchor targets or replace with plain-text references.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...de** - Tree data structure representing markdown document hierarchy with stable IDs, nes...
(MARKDOWN_NNP)
[uncategorized] ~9-~9: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...te-0) MarkdownSection - Raw parsed markdown heading with embedded IDs, content boun...
(MARKDOWN_NNP)
[uncategorized] ~15-~15: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... Level vs Depth - level refers to markdown heading level (1-6), while depth refe...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.20.0)
[warning] 7-7: Link fragments should be valid
(MD051, link-fragments)
[warning] 9-9: Link fragments should be valid
(MD051, link-fragments)
[warning] 11-11: Link fragments should be valid
(MD051, link-fragments)
[warning] 13-13: Link fragments should be valid
(MD051, link-fragments)
[warning] 15-15: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
In `@Phase_0_Chat_Refactor.md` around lines 7 - 15, The document contains broken
citation link fragments like [0-cite-N](`#0-cite-N`) across multiple sections
(e.g., in definitions for OutlineNode, MarkdownSection, MessagePair,
Scope/ScopeId, and Level vs Depth); fix by either adding matching anchor targets
(e.g., create corresponding headings or anchor IDs named 0-cite-0, 0-cite-1,
etc.) or replacing each inline link with a plain-text citation (e.g., “see
citation 0-cite-0”) for the referenced concepts (OutlineNode, MarkdownSection,
MessagePair, Scope/ScopeId, Level vs Depth) so the references resolve correctly.
AndlerRL
left a comment
There was a problem hiding this comment.
I am still reviewing. I have to check what is going on with the seed files since I see them all changed in size, suggesting either a formatting or an addition that I would like to confirm that it works, so I can be able to merge this PR since it has a few improvements that we can take, however, it also has changes that may break the chat. Important to take care the observations.
| <MemoizedReactMarkdown | ||
| className="min-w-full prose break-words dark:prose-invert prose-p:leading-relaxed prose-pre:p-0" | ||
| remarkPlugins={[remarkGfm, rehypeMathJax, remarkRehype]} | ||
| remarkPlugins={[remarkGfm]} |
There was a problem hiding this comment.
We need to review for regressions—however, I have noticed issues with a few formulas and MathJax nor Rehype was picking them hence, I had to create a custom rule to support them, but that worked partially. We need to do a deep test with different formulas. This is important due there are plenty of startups that needs maths formulas to get things done. Maths are everywhere!
| "react-markdown": "8.0.7", | ||
| "react-syntax-highlighter": "^15.6.1", | ||
| "react-textarea-autosize": "^8.5.7", | ||
| "react-use": "^17.6.0", | ||
| "rehype-katex": "^7.0.1", | ||
| "rehype-mathjax": "^7.1.0", | ||
| "remark-gfm": "4.0.1", | ||
| "remark-math": "^6.0.0", | ||
| "rehype-katex": "6.0.3", | ||
| "rehype-mathjax": "4.0.0", | ||
| "remark-gfm": "3.0.1", | ||
| "remark-math": "3.0.1", |
There was a problem hiding this comment.
Hm, this is a big version revert... Yes, we have to definitely test these plugins and markdown. Markdown wasn't having issues to begging with though, since Markdown the only issue that we had is that we made a major markdown dependency update and that caused some issues, however, was stabilised long ago and only a few of maths formulas are difficult to take.
| ], | ||
| "paths": { | ||
| "@/*": ["./*"] | ||
| "@/*": ["./*"], |
There was a problem hiding this comment.
I am not pretty sure if the tsconfig.json needs these specifications. These looks overkill since we are on a monorepo structure and, a monorepo with bun already maps these into their configuration by default, we don't need to specify anything else but only at the root package.json, which defines the package distributions, and we have to add them into our app dependencies to be able to import them with no issues.
There was a problem hiding this comment.
Let's make sure to avoid adding log files into git. We can add a condition into the .gitignore to ignore *.log files.
Overview
This PR addresses several stability and build issues encountered during development.
Changes
MarkdownandChatMessageProcomponents to ensure consistent rendering.useWorkspaceChathooks to improve chat reliability.next.config.tsandtsconfig.jsonto fix build errors and optimize configuration.react-markdown-patch.d.tsto resolve missing type definitions.Verification
Summary by Sourcery
Improve chat stability, markdown rendering, and build configuration while updating seeds and documenting a planned chat refactor.
Bug Fixes:
Enhancements:
Build:
Deployment:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Bug Fixes
Improvements