Skip to content

Drop recursion fix attempt 2#1621

Merged
ragnep merged 8 commits intomainfrom
drop-recursion-fix-attempt-2
Nov 28, 2025
Merged

Drop recursion fix attempt 2#1621
ragnep merged 8 commits intomainfrom
drop-recursion-fix-attempt-2

Conversation

@ragnep
Copy link
Copy Markdown
Contributor

@ragnep ragnep commented Nov 28, 2025

Summary by CodeRabbit

Bug Fixes

  • Enhanced event handling to prevent unintended bubbling of click, touch, and mouse events in chat link buttons
  • Improved link parsing logic to prevent recursive processing of certain link types

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 28, 2025

Walkthrough

Adds event propagation stopping to ChatItemHrefButtons to prevent event bubbling on interactive elements, and introduces a guard in SeizeLinkParser to return null for URLs containing drop parameters, preventing recursive drop-link parsing.

Changes

Cohort / File(s) Summary
Event Propagation Control
components/waves/ChatItemHrefButtons.tsx
Adds stopPropagation calls on copy button and Link elements to prevent event bubbling; adjusts copyToClipboard function to accept and propagate stop events; adds explicit type="button" attribute.
Drop Parameter Guard
helpers/SeizeLinkParser.ts
Adds guard condition in parseSeizeQuoteLink to return null when href contains drop parameter, preventing recursive quote-link parsing for drop URLs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Attention area: Verify that event propagation stops are comprehensive and don't interfere with other event handlers in the parent hierarchy for ChatItemHrefButtons.
  • Attention area: Confirm the drop parameter guard logic doesn't inadvertently skip valid quote-link scenarios or conflict with ensureStableSeizeLink behavior.

Possibly related PRs

  • drop preview url fix #1556: Refactors SeizeLinkParser's drop-link handling logic by introducing ensureStableSeizeLink and getSeizeBaseOrigin utilities, providing the infrastructure that this PR's drop-parameter guard builds upon.
  • prevent nesting #1605: Implements similar recursion prevention in the drop-link pipeline by threading currentDropId through handlers to avoid nested drop links, using a complementary approach to this PR's parameter-based guard.

Suggested reviewers

  • simo6529
  • prxt6529

Poem

🐰 A click, a stop, no bubble flies,
No recursion trips the drop's surprise!
Events held still, like whispered leaves,
Your careful guards are what the codebase needs. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title refers to fixing a recursion issue with 'Drop', which is directly addressed in the changes (adds guard in parseSeizeQuoteLink for drop parameter), but uses vague phrasing ('attempt 2') that doesn't clearly convey the specific technical nature of the fix. Consider a more specific title like 'Prevent drop recursion in seize quote link parsing' or 'Add drop parameter guard to parseSeizeQuoteLink' to clearly communicate the technical change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch drop-recursion-fix-attempt-2

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
components/waves/ChatItemHrefButtons.tsx (1)

30-40: Consider extracting the IIFE for clarity.

While the inline computation works correctly, the IIFE (lines 33-38) could be extracted into a named function to improve readability, especially given the "self-explanatory code" guideline.

+const deriveRelativeHref = (stableHref: string): string | undefined => {
+  const internal = removeBaseEndpoint(stableHref);
+  return typeof internal === "string" && internal.startsWith("/")
+    ? internal
+    : undefined;
+};
+
 export default function ChatItemHrefButtons({
   href,
   relativeHref,
   hideLink = false,
 }: {
   href: string;
   relativeHref?: string;
   hideLink?: boolean;
 }) {
   const [copied, setCopied] = useState(false);

   const stableHref = ensureStableSeizeLink(href);
-  const derivedRelativeHref =
-    relativeHref ??
-    (() => {
-      const internal = removeBaseEndpoint(stableHref);
-      return typeof internal === "string" && internal.startsWith("/")
-        ? internal
-        : undefined;
-    })();
+  const derivedRelativeHref = relativeHref ?? deriveRelativeHref(stableHref);
   const linkHref = derivedRelativeHref ?? stableHref;
   const isExternalLink = !derivedRelativeHref;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aea1847 and 3af5e3b.

📒 Files selected for processing (3)
  • __tests__/components/ChatItemHrefButtons.test.tsx (2 hunks)
  • components/waves/ChatItemHrefButtons.tsx (3 hunks)
  • helpers/SeizeLinkParser.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version

**/*.{ts,tsx,js,jsx}: Enforce ≥ 80% line coverage for files changed since main via npm run test
All code must pass ESLint (Next's Core Web Vitals + React Hooks rules) via npm run lint
Use <Link> from Next.js for internal navigation instead of <a> tags or HTML anchors
Use <Image> from next/image instead of HTML <img> elements

Files:

  • helpers/SeizeLinkParser.ts
  • __tests__/components/ChatItemHrefButtons.test.tsx
  • components/waves/ChatItemHrefButtons.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: All code must pass TypeScript type checking via npm run type-check (tsc --noEmit)
Use 'use cache' directive at the top of Server Components or functions to explicitly opt-in to caching in Next.js 16

Files:

  • helpers/SeizeLinkParser.ts
  • __tests__/components/ChatItemHrefButtons.test.tsx
  • components/waves/ChatItemHrefButtons.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always add readonly before props in React components

Files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
  • components/waves/ChatItemHrefButtons.tsx
**/__tests__/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Tests should live in __tests__/ directory or be named ComponentName.test.tsx

Files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
🧠 Learnings (7)
📚 Learning: 2025-11-25T08:37:36.715Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:36.715Z
Learning: Applies to __tests__/**/__tests__/**/*.{ts,tsx,js,jsx} : Use the Arrange – Act – Assert pattern for test structure, keeping assertions focused on behaviour, not implementation details

Applied to files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
📚 Learning: 2025-11-25T08:37:14.939Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:14.939Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `<Link>` from Next.js for internal navigation instead of `<a>` tags or HTML anchors

Applied to files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
  • components/waves/ChatItemHrefButtons.tsx
📚 Learning: 2025-11-25T08:37:36.715Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:36.715Z
Learning: Applies to __tests__/**/__tests__/**/*.{ts,tsx,js,jsx} : Give each test a clear, descriptive name that conveys the scenario and expected outcome

Applied to files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
📚 Learning: 2025-11-25T08:37:14.939Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:14.939Z
Learning: Applies to **/__tests__/**/*.{ts,tsx,js,jsx} : Tests should live in `__tests__/` directory or be named `ComponentName.test.tsx`

Applied to files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
📚 Learning: 2025-11-25T08:37:36.715Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:36.715Z
Learning: Applies to __tests__/**/__tests__/**/*.{ts,tsx,js,jsx} : Organize test files in `__tests__` directory mirroring source folder structure (components, contexts, hooks, utils) to keep structure familiar

Applied to files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
📚 Learning: 2025-11-25T08:37:36.715Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:36.715Z
Learning: Use testing-library/react and testing-library/user-event for React component tests

Applied to files:

  • __tests__/components/ChatItemHrefButtons.test.tsx
📚 Learning: 2025-11-25T08:37:14.939Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:14.939Z
Learning: Use TypeScript and React functional components with hooks

Applied to files:

  • components/waves/ChatItemHrefButtons.tsx
🧬 Code graph analysis (2)
__tests__/components/ChatItemHrefButtons.test.tsx (3)
helpers/SeizeLinkParser.ts (1)
  • ensureStableSeizeLink (160-220)
helpers/Helpers.ts (1)
  • removeBaseEndpoint (793-795)
components/waves/ChatItemHrefButtons.tsx (1)
  • ChatItemHrefButtons (19-126)
components/waves/ChatItemHrefButtons.tsx (2)
helpers/SeizeLinkParser.ts (1)
  • ensureStableSeizeLink (160-220)
helpers/Helpers.ts (1)
  • removeBaseEndpoint (793-795)
🪛 GitHub Check: SonarCloud Code Analysis
__tests__/components/ChatItemHrefButtons.test.tsx

[warning] 66-66: Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element.

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZrJvqvZgaKTI8XGFjem&open=AZrJvqvZgaKTI8XGFjem&pullRequest=1621


[warning] 66-66: Visible, non-interactive elements with click handlers must have at least one keyboard listener.

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZrJvqvZgaKTI8XGFjen&open=AZrJvqvZgaKTI8XGFjen&pullRequest=1621

components/waves/ChatItemHrefButtons.tsx

[warning] 68-72: Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element.

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZrJvqrygaKTI8XGFjel&open=AZrJvqrygaKTI8XGFjel&pullRequest=1621

🔇 Additional comments (8)
helpers/SeizeLinkParser.ts (1)

78-83: Well-designed recursion guard.

The early return when a "drop" parameter is present effectively prevents recursive parsing between quote and drop handlers. The placement after origin/pathname validation but before wave/serialNo parsing is optimal for performance.

__tests__/components/ChatItemHrefButtons.test.tsx (3)

3-9: LGTM! Proper mock initialization.

The mocks are correctly positioned before the component import, and the default identity function behavior allows for flexible test-specific overrides.


47-61: Excellent test coverage for href derivation.

This test thoroughly validates the stable link resolution logic, verifying both the transformation pipeline (ensureStableSeizeLink → removeBaseEndpoint) and the resulting internal link characteristics.


63-76: LGTM! Comprehensive propagation test.

The test correctly verifies that click events are prevented from bubbling to parent containers for both the button and link elements.

Note: The SonarCloud warning about the div on line 66 is a false positive—this is test scaffolding used to verify propagation behavior, not a production interactive element.

components/waves/ChatItemHrefButtons.tsx (4)

9-17: Robust propagation control.

The helper correctly handles multiple event types and uses optional chaining for stopImmediatePropagation, which ensures compatibility across different event contexts.


49-65: LGTM! Well-structured event handlers.

The handlers correctly implement propagation control while preserving the intended behaviors (clipboard write, link navigation).


68-72: Defensive wrapper is appropriate.

The container div with propagation handlers is a defensive pattern to prevent events from bubbling, not an interactive element. The SonarCloud warning is a false positive—the actual interactive elements (button and Link) are properly implemented inside.


95-103: LGTM! Proper Next.js Link usage with security.

The Link component correctly differentiates between internal and external navigation, applying appropriate security attributes (target="_blank" with rel="noopener noreferrer") only for external links.

Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
Signed-off-by: ragnep <ragneinfo@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
components/waves/ChatItemHrefButtons.tsx (2)

32-38: Consider adding error handling for clipboard API.

The clipboard write operation has no catch handler. If clipboard access is denied (e.g., in certain browsers or security contexts), the promise rejection will be unhandled.

  const copyToClipboard = (event: MouseEvent<HTMLButtonElement>) => {
    stopPropagation(event);
-   navigator.clipboard.writeText(href).then(() => {
-     setCopied(true);
-     setTimeout(() => setCopied(false), 500);
-   });
+   navigator.clipboard.writeText(href).then(() => {
+     setCopied(true);
+     setTimeout(() => setCopied(false), 500);
+   }).catch(() => {
+     // Silently fail or optionally show error feedback
+   });
  };

25-28: Props should be marked as readonly.

As per coding guidelines, props in React components should be prefixed with readonly.

 }: {
-  href: string;
-  relativeHref?: string;
-  hideLink?: boolean;
+  readonly href: string;
+  readonly relativeHref?: string;
+  readonly hideLink?: boolean;
 })
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93d7c53 and 7f07f52.

📒 Files selected for processing (1)
  • components/waves/ChatItemHrefButtons.tsx (5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version

**/*.{ts,tsx,js,jsx}: Enforce ≥ 80% line coverage for files changed since main via npm run test
All code must pass ESLint (Next's Core Web Vitals + React Hooks rules) via npm run lint
Use <Link> from Next.js for internal navigation instead of <a> tags or HTML anchors
Use <Image> from next/image instead of HTML <img> elements

Files:

  • components/waves/ChatItemHrefButtons.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always add readonly before props in React components

Files:

  • components/waves/ChatItemHrefButtons.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: All code must pass TypeScript type checking via npm run type-check (tsc --noEmit)
Use 'use cache' directive at the top of Server Components or functions to explicitly opt-in to caching in Next.js 16

Files:

  • components/waves/ChatItemHrefButtons.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-25T08:37:14.939Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:14.939Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `<Link>` from Next.js for internal navigation instead of `<a>` tags or HTML anchors

Applied to files:

  • components/waves/ChatItemHrefButtons.tsx
📚 Learning: 2025-11-25T08:37:14.939Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T08:37:14.939Z
Learning: Use TypeScript and React functional components with hooks

Applied to files:

  • components/waves/ChatItemHrefButtons.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
components/waves/ChatItemHrefButtons.tsx (3)

11-19: Clean utility for comprehensive event propagation control.

The StopEvent union type and the stopPropagation helper are well-designed. Using optional chaining on stopImmediatePropagation is a good defensive approach since it may not be present on all native event types.


42-48: Thorough event propagation handling.

The explicit type="button" attribute is a good addition preventing accidental form submissions. Covering onClick, onPointerDown, onMouseDown, and onTouchStart ensures propagation is stopped across all interaction types and devices.


64-71: Consistent propagation handling on Link.

Good use of Next.js Link component for navigation. The propagation stopping is consistently applied across all interaction event types, matching the button's behavior.

@ragnep ragnep merged commit 261fc9a into main Nov 28, 2025
9 checks passed
@ragnep ragnep deleted the drop-recursion-fix-attempt-2 branch November 28, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants