✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

Summary

This PR fixes a bug (gh-13339) where tooltip text containing newlines (\n\n) would "leak" into the markdown/caption content instead of rendering properly inside the tooltip popup. The issue stemmed from markdown text directives not supporting multiline content.

Solution implemented:

• Escape newlines as \\n before embedding help text in the :help[] directive (Markdown.tsx)
• Unescape \\n back to \n when rendering tooltip content in CustomHelpIcon (StreamlitMarkdown.tsx)

Code Quality

The implementation is clean, minimal, and well-targeted:

Markdown.tsx (lines 79-82):

const escapedHelp = help ? help.replace(/\n/g, "\\n") : ""
const source = help ? `${body} :help[${escapedHelp}]` : body

StreamlitMarkdown.tsx (lines 463-466):

const tooltipContent =
  typeof children === "string" ? children.replace(/\\n/g, "\n") : ""

✅ Comments clearly explain the workaround for text directive limitations
✅ Follows existing patterns in the codebase
✅ Changes are focused and minimal
✅ No unnecessary modifications

Test Coverage

E2E Tests (✅ Well covered)

The PR adds comprehensive E2E tests following best practices from .cursor/rules/e2e_playwright.mdc:

• ✅ Uses parameterized tests (@pytest.mark.parametrize) for 3 scenarios
• ✅ Uses key-based locators via get_element_by_key
• ✅ Uses stable locators (get_by_test_id)
• ✅ Uses expect for auto-wait assertions (not assert)
• ✅ Descriptive test name with issue reference (test_tooltip_with_newlines_gh_13339)
• ✅ Good docstring explaining the test purpose and bug behavior
• ✅ Tests cover st.markdown, st.caption, and center-aligned markdown
• ✅ Contains critical negative assertions (verifies text doesn't leak)
• ✅ Verifies tooltip appears and contains all expected content

Frontend Unit Tests (⚠️ Could be improved)

The existing Markdown.test.tsx has tests for help tooltips, but there's no unit test specifically for the newline escaping/unescaping behavior. While the E2E tests provide coverage, a unit test would:

• Provide faster feedback during development
• Document the expected behavior at the unit level
• Catch regressions earlier in the test pyramid

Suggested addition to Markdown.test.tsx:

it("renders markdown help tooltip with newlines correctly", async () => {
  const user = userEvent.setup()
  const props = getProps({ help: "Line 1\n\nLine 2\n\nLine 3" })
  render(<Markdown {...props} />)
  
  const tooltip = screen.getByTestId("stTooltipHoverTarget")
  await user.hover(tooltip)
  
  const helpText = await screen.findByTestId("stTooltipContent")
  expect(helpText).toHaveTextContent("Line 1")
  expect(helpText).toHaveTextContent("Line 2")
  expect(helpText).toHaveTextContent("Line 3")
})

Backwards Compatibility

✅ Fully backwards compatible

• Existing help tooltips without newlines will continue to work identically
• Tooltips with newlines will now render correctly (this is the bug fix)
• No breaking changes to the public API

Security & Risk

✅ No security concerns identified

• Changes are minimal and focused on string replacement
• The regex replacements are straightforward and safe
• No user input is being evaluated or executed unsafely
• The escaping/unescaping pattern is a well-known technique

Minor edge case to note: If a user intentionally wanted to display the literal string \n in their tooltip (e.g., help="Type \\n for newline"), this would be converted to an actual newline. This is an extremely rare use case and the current behavior (treating \n as newline) is more intuitive for most users.

Recommendations

1. Consider adding a frontend unit test for the newline handling behavior in Markdown.test.tsx. This would provide faster feedback and better document the expected behavior. (Optional - E2E tests provide adequate coverage)

2. The implementation is solid and ready for merge as-is. The E2E test coverage is comprehensive and follows all best practices.

Verdict

APPROVED: This is a well-implemented bug fix with comprehensive E2E test coverage. The changes are minimal, focused, backwards compatible, and follow the codebase's existing patterns. The PR is ready for merge.

This is an automated AI review. Please verify the feedback and use your judgment.

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0000%

• Current PR: 86.3400% (12614 lines, 1722 missed)
• Latest develop: 86.3400% (12611 lines, 1722 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

@cursor review

Summary

This PR fixes issue gh-13339 where tooltip text containing newlines (\n\n) was "leaking" into the markdown content instead of being displayed inside the tooltip popup. The root cause was that the remark text directive syntax (:help[content]) doesn't properly handle newlines or certain special characters in the directive label.

Solution: Instead of embedding the help text directly in the directive (:help[${help}]), the PR changes to use an empty marker (:help[]) and passes the actual help text via React Context (HelpTextContext). The CustomHelpIcon component then retrieves the help text from context, falling back to the directive children for backward compatibility with manual :help[text] usage.

Code Quality

The code changes are well-structured and follow existing patterns in the codebase:

Strengths

1. Clean Context Implementation (StreamlitMarkdown.tsx):

   • The HelpTextContext is properly created with a display name for React DevTools
   • The context is appropriately scoped - it wraps only the ReactMarkdown component
   • The fallback logic in CustomHelpIcon (contextHelpText || children) maintains backward compatibility

2. Good Documentation (StreamlitMarkdown.tsx, lines 471-484):

   • The JSDoc comment on CustomHelpIcon clearly explains the limitations of the text directive approach and when to use context vs. directive children

3. Minimal Surface Area: The changes are focused and don't introduce unnecessary complexity. Only the required components (Markdown.tsx, StreamlitMarkdown.tsx) are modified.

4. Proper Prop Threading: The helpText prop is correctly added to both Props and RenderedMarkdownProps interfaces with appropriate documentation.

Minor Observations

1. Line 1018 in StreamlitMarkdown.tsx: The context provider wrapping is clean, but consider whether wrapping the ErrorBoundary inside the provider (current) or outside makes semantic sense. The current approach is fine since the context is only needed by children of ReactMarkdown.

Test Coverage

The test coverage is excellent and comprehensive:

Unit Tests (Markdown.test.tsx)

✅ 7 new test cases covering:

• Basic newlines (Line 1\n\nLine 2\n\nLine 3)
• Literal backslash-n (Use \\n for newlines)
• Spaces around newlines (Tool tip with \n\n new lines)
• Closing bracket in tooltip (Help before ] help after)
• Inline code (Use \st.markdown()` for text`)
• Complex markdown (code, brackets, newlines, bold)

All tests properly:

• Verify content is NOT leaked into the main markdown body
• Verify tooltip appears on hover
• Verify tooltip contains expected content

E2E Tests (st_markdown_test.py)

✅ 2 new test functions with 6 parameterized test cases:

1. test_tooltip_with_newlines_gh_13339 - Parameterized test covering:

   • st.markdown with newlines
   • st.caption with newlines
   • Center-aligned markdown with newlines
   • Markdown with spaces around newlines
   • Markdown with closing brackets

2. test_tooltip_with_complex_markdown_gh_13339 - Comprehensive test with snapshot

E2E Best Practices Compliance:

• ✅ Uses get_element_by_key() with container wrappers (following priority Fix line number in error message #3 from e2e_playwright.mdc)
• ✅ Uses expect() for auto-wait assertions
• ✅ Uses descriptive test names with issue reference (Tooltip shows text outside of the box when there are new lines in the help text #13339)
• ✅ Has proper docstrings explaining the test purpose
• ✅ Uses scroll_into_view_if_needed() before interactions
• ✅ Snapshot test targets specific element (tooltip_content), not whole page
• ✅ Follows naming convention: st_markdown-complex_tooltip_with_markdown_formatting

Test App (st_markdown.py)

✅ Test cases added with unique container keys for reliable targeting:

• markdown_newlines_tooltip
• caption_newlines_tooltip
• markdown_center_newlines_tooltip
• markdown_spaces_around_newlines
• markdown_bracket_in_tooltip
• markdown_complex_tooltip

Backwards Compatibility

This PR is fully backwards compatible:

1. Manual :help[text] usage preserved: The CustomHelpIcon component falls back to children (directive label content) when contextHelpText is not available. This means any existing manual usage of :help[simple text] in markdown will continue to work.

2. API unchanged: The help parameter on st.markdown() and st.caption() works exactly the same from the user's perspective.

3. No breaking changes to component interfaces: The new helpText prop is optional on all interfaces.

Security & Risk

No security concerns identified:

1. The help text is passed through the same rendering pipeline as before - just via a different mechanism (context vs. directive label).

2. The InlineTooltipIcon component that receives the help text has not been modified and maintains its existing security posture.

3. No new user-controlled data paths or XSS vectors introduced.

Low regression risk:

1. The change is additive - context is used only when available, with fallback to existing behavior.

2. The fix targets a specific bug without modifying core markdown parsing logic.

3. Comprehensive test coverage reduces regression risk.

Recommendations

The PR is well-implemented with no blocking issues. A few minor suggestions:

1. Consider adding a unit test for empty help text: While edge case, testing help="" behavior could be valuable to ensure no unexpected rendering occurs.

2. Documentation note: The JSDoc comment on CustomHelpIcon is excellent. Consider adding a brief inline comment in Markdown.tsx (around line 81) explaining why the empty marker pattern is used:

   // Use :help[] as marker only; actual content passed via helpText prop
   // to avoid text directive limitations with newlines and special chars
   const source = help ? `${body} :help[]` : body

   (This is already present and is good - just noting it's valuable.)

Verdict

APPROVED: This is a well-implemented bug fix with comprehensive test coverage, proper backwards compatibility handling, and no security concerns. The solution elegantly works around text directive parsing limitations by using React Context, which is an appropriate pattern for this use case.

This is an automated AI review. Please verify the feedback and use your judgment.