Conversation
📝 WalkthroughWalkthroughAdds a renderer registry and variant resolver for wave participation UI, introduces quorum proposal parsing and a compact proposal UI, threads a new contentPresentation prop through drop components, and refactors renderer selection to use centralized hooks. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ParticipationDrop
participant Registry as useWaveParticipationRendererSet
participant QuorumDrop as QuorumParticipationDrop
participant RulesHook as useDropInteractionRules
participant MarkdownComp as WaveDropPartContentMarkdown
participant Parser as parseQuorumProposalMarkdown
participant CompactUI as QuorumProposalCompactContent
Client->>ParticipationDrop: render(drop)
ParticipationDrop->>Registry: useWaveParticipationRendererSet(wave.id)
Registry-->>ParticipationDrop: { variant, ParticipationDrop, SingleWaveDrop }
ParticipationDrop->>ParticipationDrop: choose renderer
alt variant == "quorum"
ParticipationDrop->>QuorumDrop: render ParticipationDrop (props)
QuorumDrop->>RulesHook: useDropInteractionRules(drop)
RulesHook-->>QuorumDrop: { isVotingEnded }
QuorumDrop->>QuorumDrop: pick Ended/Ongoing with contentPresentation="quorumCompact"
QuorumDrop->>MarkdownComp: render parts with contentPresentation="quorumCompact"
MarkdownComp->>Parser: parseQuorumProposalMarkdown(part.content)
Parser-->>MarkdownComp: parsed proposal
MarkdownComp->>CompactUI: render parsed compact UI
else
ParticipationDrop->>ParticipationDrop: render resolved ParticipationDrop
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01e1f54d0b
ℹ️ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
__tests__/helpers/waves/wave-participation-presentation.helpers.test.ts (1)
34-43: Add a normalization-focused override case.Since the helper normalizes
waveIdbefore lookup, one test with whitespace/case-normalized input would protect the override contract.Test coverage addition
it("prefers explicit overrides over built-in variants", () => { expect( resolveWaveParticipationVariant({ waveId: "meme-wave", overrides: { "meme-wave": "quorum" }, isMemesWave: (waveId) => waveId === "meme-wave", isQuorumWave: () => false, }) ).toBe("quorum"); }); + + it("looks up overrides using the normalized wave id", () => { + expect( + resolveWaveParticipationVariant({ + waveId: " meme-wave ", + overrides: { "meme-wave": "quorum" }, + isMemesWave: () => false, + isQuorumWave: () => false, + }) + ).toBe("quorum"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/helpers/waves/wave-participation-presentation.helpers.test.ts` around lines 34 - 43, Add a test to verify resolveWaveParticipationVariant respects overrides after normalizing waveId: call resolveWaveParticipationVariant with an overrides map containing the normalized key (e.g., "meme-wave") but pass a waveId with different case/extra whitespace (e.g., " Meme-Wave "), and assert the function returns the override value ("quorum"); reference resolveWaveParticipationVariant and the overrides parameter to locate where to add this normalization-focused test alongside the existing cases.components/waves/drops/WaveDropPartContentMarkdown.tsx (1)
64-67: Memoize compact proposal parsing.
parseQuorumProposalMarkdowncan run on every render for quorum drops, including edit-mode renders where the result is unused. Memoizing also skips the parse while editing.♻️ Proposed refactor
- const compactProposal = - contentPresentation === "quorumCompact" - ? parseQuorumProposalMarkdown(part.content) - : null; + const compactProposal = React.useMemo( + () => + !isEditing && contentPresentation === "quorumCompact" + ? parseQuorumProposalMarkdown(part.content) + : null, + [contentPresentation, isEditing, part.content] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drops/WaveDropPartContentMarkdown.tsx` around lines 64 - 67, The compactProposal is being re-parsed on every render; wrap the parseQuorumProposalMarkdown call in a React useMemo so it only runs when needed by depending on contentPresentation === "quorumCompact" and part.content (and also the component's edit-mode flag if one exists, e.g., isEditing or isEditMode) — compute parseQuorumProposalMarkdown(part.content) only when contentPresentation is "quorumCompact" and not in edit mode, using a dependency array like [contentPresentation, part.content, isEditing] (omit isEditing if not present).components/waves/quorum/QuorumProposalCompactContent.tsx (1)
53-57: Narrow the keyboard propagation guard.Stopping every
keydownat the section container can block parent Escape/shortcut handlers. Keep the parent-drop protection, but limit keyboard propagation stopping to the keys that toggle<summary>.♻️ Proposed refinement
<details open={isOpen} onToggle={(event) => setIsOpen(event.currentTarget.open)} onClick={(event) => event.stopPropagation()} - onKeyDown={(event) => event.stopPropagation()} + onKeyDown={(event) => { + if (event.key === "Enter" || event.key === " ") { + event.stopPropagation(); + } + }} className="tw-rounded-xl tw-border tw-border-solid tw-border-iron-800 tw-bg-iron-950/70" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/quorum/QuorumProposalCompactContent.tsx` around lines 53 - 57, The current onKeyDown handler on the details container stops all keydown propagation which blocks parent-level shortcuts (e.g., Escape); change the onKeyDown on the <details> to only stop propagation for the keys that actually toggle a <summary> (e.g., "Enter" and " " / "Spacebar") so other keys bubble normally; keep the existing onClick stopPropagation for mouse-driven parent-drop protection and continue using isOpen/setIsOpen as before.components/waves/quorum/quorumProposalMarkdown.ts (2)
451-452: Nit: useArray#at(-1)for last-element access.Minor readability improvement per the SonarCloud hint; no behavior change.
♻️ Proposed tweak
- const lastCandidateIndex = - candidatePathIndexes[candidatePathIndexes.length - 1]; + const lastCandidateIndex = candidatePathIndexes.at(-1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/quorum/quorumProposalMarkdown.ts` around lines 451 - 452, Replace the manual last-element access for candidatePathIndexes with Array#at to improve readability: change the expression that sets lastCandidateIndex (currently using candidatePathIndexes[candidatePathIndexes.length - 1]) to use candidatePathIndexes.at(-1) so the code reads and intent clearer while preserving behavior.
439-530: Optional: bound DFS blow-up on pathological inputs.
selectSectionBoundaryCandidatesruns a backtracking DFS over candidate paths. Paths are strictly increasing inheadingIndex(bounded length 8), so well-formed proposals are fine. However, adversarially crafted markdown that repeats canonical headings many times (e.g., hundreds of## Summary/## Problem Statementlines) can exponentially grow the explored state — each step also spreadscandidatePathIndexesand allocates a freshSet. SinceparseQuorumProposalMarkdownis invoked on user-submitted proposal content (quorum compact rendering path), consider either an early-exit bound on visited states or capping the input size upstream. Not a release blocker for realistic proposals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/quorum/quorumProposalMarkdown.ts` around lines 439 - 530, selectSectionBoundaryCandidates performs an unbounded backtracking DFS (visitCandidatePath) that can explode on adversarial input; add a defensive cap and/or memoization to stop pathological work: implement a MAX_DFS_STEPS counter (or MAX_CANDIDATES) at the top of selectSectionBoundaryCandidates and early-return when exceeded, and/or maintain a visited Set/Map keyed by (lastCandidateIndex, nextCandidateIndex, serialized requiredFutureHeadingIndexes) to skip revisiting identical states before recursing; update visitCandidatePath to increment the step counter and check the visited map before calling buildNextRequiredFutureHeadingIndexes/updateBestCandidatePath so expensive allocations are avoided on repeated states. Ensure the new limit and memoization are used in selectSectionBoundaryCandidates/visitCandidatePath and mention constants in tests if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helpers/waves/wave-participation-presentation.helpers.ts`:
- Around line 3-21: The resolver resolveWaveParticipationVariant currently only
checks isMemesWave and isQuorumWave and thus maps curation waves to the
"default" renderer; either document that behavior or add explicit curation
support by extending WaveParticipationVariant with a "curation" variant, update
DEFAULT_WAVE_PARTICIPATION_VARIANT_OVERRIDES if needed, and wire the new
predicate isCurationWave (from useSeizeSettings()) into
resolveWaveParticipationVariant so curation waves return "curation" instead of
falling through to "default".
---
Nitpick comments:
In `@__tests__/helpers/waves/wave-participation-presentation.helpers.test.ts`:
- Around line 34-43: Add a test to verify resolveWaveParticipationVariant
respects overrides after normalizing waveId: call
resolveWaveParticipationVariant with an overrides map containing the normalized
key (e.g., "meme-wave") but pass a waveId with different case/extra whitespace
(e.g., " Meme-Wave "), and assert the function returns the override value
("quorum"); reference resolveWaveParticipationVariant and the overrides
parameter to locate where to add this normalization-focused test alongside the
existing cases.
In `@components/waves/drops/WaveDropPartContentMarkdown.tsx`:
- Around line 64-67: The compactProposal is being re-parsed on every render;
wrap the parseQuorumProposalMarkdown call in a React useMemo so it only runs
when needed by depending on contentPresentation === "quorumCompact" and
part.content (and also the component's edit-mode flag if one exists, e.g.,
isEditing or isEditMode) — compute parseQuorumProposalMarkdown(part.content)
only when contentPresentation is "quorumCompact" and not in edit mode, using a
dependency array like [contentPresentation, part.content, isEditing] (omit
isEditing if not present).
In `@components/waves/quorum/QuorumProposalCompactContent.tsx`:
- Around line 53-57: The current onKeyDown handler on the details container
stops all keydown propagation which blocks parent-level shortcuts (e.g.,
Escape); change the onKeyDown on the <details> to only stop propagation for the
keys that actually toggle a <summary> (e.g., "Enter" and " " / "Spacebar") so
other keys bubble normally; keep the existing onClick stopPropagation for
mouse-driven parent-drop protection and continue using isOpen/setIsOpen as
before.
In `@components/waves/quorum/quorumProposalMarkdown.ts`:
- Around line 451-452: Replace the manual last-element access for
candidatePathIndexes with Array#at to improve readability: change the expression
that sets lastCandidateIndex (currently using
candidatePathIndexes[candidatePathIndexes.length - 1]) to use
candidatePathIndexes.at(-1) so the code reads and intent clearer while
preserving behavior.
- Around line 439-530: selectSectionBoundaryCandidates performs an unbounded
backtracking DFS (visitCandidatePath) that can explode on adversarial input; add
a defensive cap and/or memoization to stop pathological work: implement a
MAX_DFS_STEPS counter (or MAX_CANDIDATES) at the top of
selectSectionBoundaryCandidates and early-return when exceeded, and/or maintain
a visited Set/Map keyed by (lastCandidateIndex, nextCandidateIndex, serialized
requiredFutureHeadingIndexes) to skip revisiting identical states before
recursing; update visitCandidatePath to increment the step counter and check the
visited map before calling
buildNextRequiredFutureHeadingIndexes/updateBestCandidatePath so expensive
allocations are avoided on repeated states. Ensure the new limit and memoization
are used in selectSectionBoundaryCandidates/visitCandidatePath and mention
constants in tests if needed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f6186d9-1714-4f1c-be64-563f6cfcb624
📒 Files selected for processing (25)
__tests__/components/waves/drop/SingleWaveDrop.test.tsx__tests__/components/waves/drops/WaveDropPartContentMarkdown.test.tsx__tests__/components/waves/drops/participation/ParticipationDrop.test.tsx__tests__/components/waves/quorum/QuorumParticipationDrop.test.tsx__tests__/components/waves/quorum/QuorumProposalCompactContent.test.tsx__tests__/components/waves/quorum/quorumProposalMarkdown.test.ts__tests__/helpers/waves/wave-participation-presentation.helpers.test.tscomponents/waves/drop/QuorumSingleWaveDrop.tsxcomponents/waves/drop/SingleWaveDrop.tsxcomponents/waves/drops/WaveDropContent.tsxcomponents/waves/drops/WaveDropPart.tsxcomponents/waves/drops/WaveDropPartContent.tsxcomponents/waves/drops/WaveDropPartContentMarkdown.tsxcomponents/waves/drops/WaveDropPartDrop.tsxcomponents/waves/drops/dropContentPresentation.tscomponents/waves/drops/participation/EndedParticipationDrop.tsxcomponents/waves/drops/participation/OngoingParticipationDrop.tsxcomponents/waves/drops/participation/ParticipationDrop.tsxcomponents/waves/drops/participation/ParticipationDropContent.tsxcomponents/waves/drops/participation/participationRenderer.types.tscomponents/waves/drops/participation/participationRendererRegistry.tsxcomponents/waves/quorum/QuorumParticipationDrop.tsxcomponents/waves/quorum/QuorumProposalCompactContent.tsxcomponents/waves/quorum/quorumProposalMarkdown.tshelpers/waves/wave-participation-presentation.helpers.ts
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
__tests__/components/waves/drops/participation/participationRendererRegistry.test.tsx (1)
46-69: LGTM — consider extending coverage.The curation-variant case is wired correctly against the mocked predicates and the registry mapping. Since the resolver has four branches (memes/curation/quorum/default) plus the overrides map and null/undefined
waveIdfallback, consider adding parallel cases formemes,quorum,default(all predicates false), and the overrides path to lock in the full resolver contract. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/drops/participation/participationRendererRegistry.test.tsx` around lines 46 - 69, Add tests covering the remaining branches of useWaveParticipationRendererSet: create separate it blocks that mockUseSeizeSettings to return isMemesWave true (assert variant === "memes" and default renderers), isQuorumWave true (assert variant === "quorum"), all predicates false (assert default variant and DefaultParticipationDrop/DefaultSingleWaveDrop), and tests for overrides and null/undefined waveId (call useWaveParticipationRendererSet with an override key and with undefined/null waveId and assert the resolver returns the override renderers or falls back to defaults). Use the same renderHook pattern as the existing curation test and reference the same symbols: useWaveParticipationRendererSet, mockUseSeizeSettings, DefaultParticipationDrop, DefaultSingleWaveDrop, and any override map keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@__tests__/components/waves/drops/participation/participationRendererRegistry.test.tsx`:
- Around line 46-69: Add tests covering the remaining branches of
useWaveParticipationRendererSet: create separate it blocks that
mockUseSeizeSettings to return isMemesWave true (assert variant === "memes" and
default renderers), isQuorumWave true (assert variant === "quorum"), all
predicates false (assert default variant and
DefaultParticipationDrop/DefaultSingleWaveDrop), and tests for overrides and
null/undefined waveId (call useWaveParticipationRendererSet with an override key
and with undefined/null waveId and assert the resolver returns the override
renderers or falls back to defaults). Use the same renderHook pattern as the
existing curation test and reference the same symbols:
useWaveParticipationRendererSet, mockUseSeizeSettings, DefaultParticipationDrop,
DefaultSingleWaveDrop, and any override map keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: deeddb33-c7b0-4ead-866a-e9f040f4ff56
📒 Files selected for processing (4)
__tests__/components/waves/drops/participation/participationRendererRegistry.test.tsx__tests__/helpers/waves/wave-participation-presentation.helpers.test.tscomponents/waves/drops/participation/participationRendererRegistry.tsxhelpers/waves/wave-participation-presentation.helpers.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/helpers/waves/wave-participation-presentation.helpers.test.ts
- components/waves/drops/participation/participationRendererRegistry.tsx
- helpers/waves/wave-participation-presentation.helpers.ts
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/components/waves/small-leaderboard/WaveSmallLeaderboardTopThreeDrop.test.tsx (1)
355-368:⚠️ Potential issue | 🟡 MinorDead spread expression —
undefined !== undefinedis always false.Biome flagged this as a self-compare. The conditional spread
...(undefined !== undefined ? { winning_context: undefined } : {})always evaluates to{}, so the test isn't actually exercising the "missing winning context" path as the name suggests. It only works because the base mock'swinning_context.decision_timehappens to fall through thedecisionTime ? ...ternary in theWinnerDropBadgemock—exceptdecision_time: 1234567890is truthy, so the assertion on Line 367 ("Winner Badge - Rank: 1"without time) would actually fail with the currentcreateMockDropdefault.🔧 Proposed fix
it("handles missing winning context gracefully", () => { - const drop = createMockDrop({ - ...(undefined !== undefined ? { winning_context: undefined } : {}), - }); + const drop = createMockDrop({ winning_context: undefined }); render( <WaveSmallLeaderboardTopThreeDrop drop={drop} wave={mockWave} onDropClick={mockOnDropClick} /> ); expect(screen.getByText("Winner Badge - Rank: 1")).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/small-leaderboard/WaveSmallLeaderboardTopThreeDrop.test.tsx` around lines 355 - 368, The test's spread uses `undefined !== undefined` which is always false so `createMockDrop` never gets `winning_context: undefined`; update the test to explicitly pass winning_context: undefined (e.g., createMockDrop({ winning_context: undefined })) so WaveSmallLeaderboardTopThreeDrop actually receives a missing winning context and the assertion for the WinnerBadge output (render of WaveSmallLeaderboardTopThreeDrop / WinnerDropBadge) exercises the intended branch.
🧹 Nitpick comments (6)
__tests__/components/WaveSmallLeaderboardDrop.test.tsx (1)
4-9: Strengthen the registry mock naming convention and assert prop forwarding to the renderer.The test verifies renderer selection but lacks assertions that
dropandonDropClickare forwarded to the resolvedSmallLeaderboardDropcomponent. Additionally, the mock variable should follow themock*naming convention for clarity with Jest's hoisting behavior.Proposed test hardening
-const useWaveLeaderboardRendererSet = jest.fn(); +const mockUseWaveLeaderboardRendererSet = jest.fn(); jest.mock("@/components/waves/leaderboard/leaderboardRendererRegistry", () => ({ useWaveLeaderboardRendererSet: (...args: any[]) => - useWaveLeaderboardRendererSet(...args), + mockUseWaveLeaderboardRendererSet(...args), })); beforeEach(() => { - useWaveLeaderboardRendererSet.mockReset(); + mockUseWaveLeaderboardRendererSet.mockReset(); }); it("renders the resolved small leaderboard renderer", () => { - useWaveLeaderboardRendererSet.mockReturnValue({ + let rendererProps: any; + mockUseWaveLeaderboardRendererSet.mockReturnValue({ variant: "quorum", LeaderboardDrop: () => null, - SmallLeaderboardDrop: () => <div>quorum</div>, + SmallLeaderboardDrop: (props: any) => { + rendererProps = props; + return <div>quorum</div>; + }, }); render( <WaveSmallLeaderboardDrop drop={drop} wave={wave} onDropClick={onDropClick} /> ); - expect(useWaveLeaderboardRendererSet).toHaveBeenCalledWith("w1"); + expect(mockUseWaveLeaderboardRendererSet).toHaveBeenCalledWith("w1"); + expect(rendererProps).toEqual({ drop, onDropClick }); expect(screen.getByText("quorum")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/WaveSmallLeaderboardDrop.test.tsx` around lines 4 - 9, Rename the jest mock variable to follow Jest convention (e.g., mockUseWaveLeaderboardRendererSet) and update the mock implementation of useWaveLeaderboardRendererSet to return the resolved renderer while preserving prop forwarding; then extend the test to render WaveSmallLeaderboardDrop and assert that the resolved SmallLeaderboardDrop was called/received props with the expected drop and onDropClick values (check mockUseWaveLeaderboardRendererSet or the resolved SmallLeaderboardDrop mock to verify it received the drop and onDropClick props).__tests__/components/waves/leaderboard/drops/QuorumWaveLeaderboardDrop.test.tsx (1)
5-17: Consider using amock*-prefixed variable name for test doubles captured by jest.mock factories.Renaming
defaultWaveLeaderboardDroptomockDefaultWaveLeaderboardDropimproves clarity and aligns with common testing conventions. While the current pattern works correctly with ts-jest, the explicitmock*prefix makes the intent of the variable clearer at a glance.This affects the variable at line 5 and its usage at line 17 and line 26.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/leaderboard/drops/QuorumWaveLeaderboardDrop.test.tsx` around lines 5 - 17, Rename the test double variable defaultWaveLeaderboardDrop to mockDefaultWaveLeaderboardDrop and update all references: the jest.fn() declaration, the jest.mock factory which returns DefaultWaveLeaderboardDrop, and calls like defaultWaveLeaderboardDrop.mockClear() and any invocations/default assertions so they use mockDefaultWaveLeaderboardDrop instead; this keeps the mock naming convention consistent and clarifies its role in the QuorumWaveLeaderboardDrop tests.__tests__/components/waves/leaderboard/content/WaveLeaderboardDropContent.test.tsx (1)
8-17: Adoptmock*naming convention for variables used in jest.mock factories.
waveDropContentMockis referenced within ajest.mockfactory, which hoists that factory to module initialization. Following the codebase convention (used consistently withmockCreateWalletClient,redirectMock, etc.), rename tomockWaveDropContentfor clarity and consistency.Rename throughout test
-const waveDropContentMock = jest.fn((props: any) => ( +const mockWaveDropContent = jest.fn((props: any) => ( <div data-testid="content" onClick={() => props.onDropContentClick(props.drop)} /> )); jest.mock("@/components/waves/drops/WaveDropContent", () => ({ __esModule: true, - default: (props: any) => waveDropContentMock(props), + default: (props: any) => mockWaveDropContent(props), })); @@ beforeEach(() => { - waveDropContentMock.mockClear(); + mockWaveDropContent.mockClear(); }); @@ - expect(waveDropContentMock).toHaveBeenCalledWith( + expect(mockWaveDropContent).toHaveBeenCalledWith(Also applies to: 38-40, 78-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/leaderboard/content/WaveLeaderboardDropContent.test.tsx` around lines 8 - 17, Rename the jest mock factory variable waveDropContentMock to mockWaveDropContent everywhere it's declared and used: update the const declaration (currently jest.fn((props: any) => ...)) to const mockWaveDropContent, and change the jest.mock factory default implementation to call mockWaveDropContent(props) instead of waveDropContentMock(props); also update any other references in the same test file (including the other occurrences around lines noted) to use mockWaveDropContent to follow the project's mock* naming convention.__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrop.test.tsx (1)
5-10: Rename the test double to improve mock naming consistency.The
mock*prefix convention clarifies intent and improves code consistency across test files. While the current pattern works correctly with ts-jest, using a mock-prefixed name follows established testing best practices.Proposed refactor
-const useWaveLeaderboardRendererSet = jest.fn(); +const mockUseWaveLeaderboardRendererSet = jest.fn(); jest.mock("@/components/waves/leaderboard/leaderboardRendererRegistry", () => ({ useWaveLeaderboardRendererSet: (...args: any[]) => - useWaveLeaderboardRendererSet(...args), + mockUseWaveLeaderboardRendererSet(...args), })); @@ beforeEach(() => { - useWaveLeaderboardRendererSet.mockReset(); + mockUseWaveLeaderboardRendererSet.mockReset(); onDropClick.mockReset(); }); @@ - useWaveLeaderboardRendererSet.mockReturnValue({ + mockUseWaveLeaderboardRendererSet.mockReturnValue({ @@ - expect(useWaveLeaderboardRendererSet).toHaveBeenCalledWith("w1"); + expect(mockUseWaveLeaderboardRendererSet).toHaveBeenCalledWith("w1");Also applies to: 17-19, 25-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrop.test.tsx` around lines 5 - 10, Rename the test double to follow the mock* convention: change the variable useWaveLeaderboardRendererSet to mockUseWaveLeaderboardRendererSet and update the jest.mock implementation to call mockUseWaveLeaderboardRendererSet(...args) instead of useWaveLeaderboardRendererSet(...args); also update every other reference to the test double (including the occurrences mentioned around the other blocks where useWaveLeaderboardRendererSet is used) to use the new name so the mock wiring for useWaveLeaderboardRendererSet in leaderboardRendererRegistry remains correct.components/waves/leaderboard/leaderboardRendererRegistry.tsx (2)
43-53: Nit:waveprop is silently dropped for Default/Quorum renderers.
DefaultLeaderboardDropRendererandQuorumLeaderboardDropRendereracceptWaveLeaderboardDropRendererProps(which includeswave) but only usedropandonDropClick. This is fine today since the underlying components don't needwave, but it's easy to miss if those components later need wave context. Consider a brief comment or explicitly destructuringwavewith avoid wave;/ ignoring via_waveto document intent — purely optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/leaderboardRendererRegistry.tsx` around lines 43 - 53, DefaultLeaderboardDropRenderer and QuorumLeaderboardDropRenderer accept WaveLeaderboardDropRendererProps which include a wave prop but currently destructure only drop and onDropClick, silently dropping wave; explicitly acknowledge and ignore wave to document intent by including it in the parameter destructuring (e.g., destructure wave as _wave or use void wave) or add a short inline comment in the function bodies referencing wave so future readers know it was intentionally unused; update the parameter lists for DefaultLeaderboardDropRenderer and QuorumLeaderboardDropRenderer accordingly.
39-41: Unused overrides map — consider inlining or removing.
WAVE_LEADERBOARD_VARIANT_OVERRIDESis always empty andresolveWaveParticipationVariantalready has aDEFAULT_WAVE_PARTICIPATION_VARIANT_OVERRIDESdefault. Keeping it is harmless and preserves an extension point, but if no override is planned in the near term, omitting the argument would reduce noise. Optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/leaderboardRendererRegistry.tsx` around lines 39 - 41, WAVE_LEADERBOARD_VARIANT_OVERRIDES is an unused empty map; remove the constant and stop passing it into resolveWaveParticipationVariant (which already uses DEFAULT_WAVE_PARTICIPATION_VARIANT_OVERRIDES), or if you want to keep the extension point, replace the explicit empty constant by inlining the default (i.e., call resolveWaveParticipationVariant without the extra overrides parameter). Update all references to WAVE_LEADERBOARD_VARIANT_OVERRIDES so only resolveWaveParticipationVariant and DEFAULT_WAVE_PARTICIPATION_VARIANT_OVERRIDES remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/waves/quorum/quorumProposalMarkdown.ts`:
- Around line 446-508: The recursive search in visitCandidatePath can explode on
inputs with many repeated headings; add memoization or bounds to prevent
combinatorial recursion: implement a DP cache keyed by (current candidate index,
nextCandidateIndex, requiredFutureHeadingIndexes bitset or serialized key) and
short-circuit when a state was already visited, or enforce a safe cap on
recursion/branching (e.g., maximum candidates explored) and return null to
trigger the fallback; update visitCandidatePath callers (and related helpers
like buildNextRequiredFutureHeadingIndexes, shouldPruneCandidatePath, and
updateBestCandidatePath) to accept/propagate the cache or early-fail signal and
ensure bestPath handling remains correct when returning null.
- Around line 263-281: The comparator compareCandidatePaths currently resolves
ties among equal-length candidate paths by using the later lineIndex, which can
incorrectly choose a generated duplicate heading as the boundary; change its
return type to number | null and when left.length === right.length return null
to signal ambiguity (instead of computing/returning a lineDifference), so
callers can fall back to normal markdown rendering; apply the same change to the
duplicate comparator at the other occurrence (lines ~399-405), update callers to
handle null fallback accordingly, and reference the function name
compareCandidatePaths to locate and update both implementations and their call
sites.
---
Outside diff comments:
In
`@__tests__/components/waves/small-leaderboard/WaveSmallLeaderboardTopThreeDrop.test.tsx`:
- Around line 355-368: The test's spread uses `undefined !== undefined` which is
always false so `createMockDrop` never gets `winning_context: undefined`; update
the test to explicitly pass winning_context: undefined (e.g., createMockDrop({
winning_context: undefined })) so WaveSmallLeaderboardTopThreeDrop actually
receives a missing winning context and the assertion for the WinnerBadge output
(render of WaveSmallLeaderboardTopThreeDrop / WinnerDropBadge) exercises the
intended branch.
---
Nitpick comments:
In
`@__tests__/components/waves/leaderboard/content/WaveLeaderboardDropContent.test.tsx`:
- Around line 8-17: Rename the jest mock factory variable waveDropContentMock to
mockWaveDropContent everywhere it's declared and used: update the const
declaration (currently jest.fn((props: any) => ...)) to const
mockWaveDropContent, and change the jest.mock factory default implementation to
call mockWaveDropContent(props) instead of waveDropContentMock(props); also
update any other references in the same test file (including the other
occurrences around lines noted) to use mockWaveDropContent to follow the
project's mock* naming convention.
In
`@__tests__/components/waves/leaderboard/drops/QuorumWaveLeaderboardDrop.test.tsx`:
- Around line 5-17: Rename the test double variable defaultWaveLeaderboardDrop
to mockDefaultWaveLeaderboardDrop and update all references: the jest.fn()
declaration, the jest.mock factory which returns DefaultWaveLeaderboardDrop, and
calls like defaultWaveLeaderboardDrop.mockClear() and any invocations/default
assertions so they use mockDefaultWaveLeaderboardDrop instead; this keeps the
mock naming convention consistent and clarifies its role in the
QuorumWaveLeaderboardDrop tests.
In `@__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrop.test.tsx`:
- Around line 5-10: Rename the test double to follow the mock* convention:
change the variable useWaveLeaderboardRendererSet to
mockUseWaveLeaderboardRendererSet and update the jest.mock implementation to
call mockUseWaveLeaderboardRendererSet(...args) instead of
useWaveLeaderboardRendererSet(...args); also update every other reference to the
test double (including the occurrences mentioned around the other blocks where
useWaveLeaderboardRendererSet is used) to use the new name so the mock wiring
for useWaveLeaderboardRendererSet in leaderboardRendererRegistry remains
correct.
In `@__tests__/components/WaveSmallLeaderboardDrop.test.tsx`:
- Around line 4-9: Rename the jest mock variable to follow Jest convention
(e.g., mockUseWaveLeaderboardRendererSet) and update the mock implementation of
useWaveLeaderboardRendererSet to return the resolved renderer while preserving
prop forwarding; then extend the test to render WaveSmallLeaderboardDrop and
assert that the resolved SmallLeaderboardDrop was called/received props with the
expected drop and onDropClick values (check mockUseWaveLeaderboardRendererSet or
the resolved SmallLeaderboardDrop mock to verify it received the drop and
onDropClick props).
In `@components/waves/leaderboard/leaderboardRendererRegistry.tsx`:
- Around line 43-53: DefaultLeaderboardDropRenderer and
QuorumLeaderboardDropRenderer accept WaveLeaderboardDropRendererProps which
include a wave prop but currently destructure only drop and onDropClick,
silently dropping wave; explicitly acknowledge and ignore wave to document
intent by including it in the parameter destructuring (e.g., destructure wave as
_wave or use void wave) or add a short inline comment in the function bodies
referencing wave so future readers know it was intentionally unused; update the
parameter lists for DefaultLeaderboardDropRenderer and
QuorumLeaderboardDropRenderer accordingly.
- Around line 39-41: WAVE_LEADERBOARD_VARIANT_OVERRIDES is an unused empty map;
remove the constant and stop passing it into resolveWaveParticipationVariant
(which already uses DEFAULT_WAVE_PARTICIPATION_VARIANT_OVERRIDES), or if you
want to keep the extension point, replace the explicit empty constant by
inlining the default (i.e., call resolveWaveParticipationVariant without the
extra overrides parameter). Update all references to
WAVE_LEADERBOARD_VARIANT_OVERRIDES so only resolveWaveParticipationVariant and
DEFAULT_WAVE_PARTICIPATION_VARIANT_OVERRIDES remain.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8c46ba5-192a-412f-9e55-25bbc89a7213
📒 Files selected for processing (30)
__tests__/components/WaveSmallLeaderboardDrop.test.tsx__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx__tests__/components/waves/leaderboard/content/WaveLeaderboardDropContent.test.tsx__tests__/components/waves/leaderboard/drops/QuorumWaveLeaderboardDrop.test.tsx__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrop.test.tsx__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrops.test.tsx__tests__/components/waves/memes/MemesArtSubmissionModal.test.tsx__tests__/components/waves/memes/submission/MemesArtResubmitAction.test.tsx__tests__/components/waves/quorum/QuorumProposalCompactContent.test.tsx__tests__/components/waves/small-leaderboard/QuorumWaveSmallLeaderboardDrop.test.tsx__tests__/components/waves/small-leaderboard/WaveSmallLeaderboardItemContent.test.tsx__tests__/components/waves/small-leaderboard/WaveSmallLeaderboardTopThreeDrop.test.tsxcomponents/brain/my-stream/MyStreamWaveLeaderboard.tsxcomponents/memes/drops/MemesLeaderboardDrop.tsxcomponents/waves/leaderboard/content/WaveLeaderboardDropContent.tsxcomponents/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.tsxcomponents/waves/leaderboard/drops/QuorumWaveLeaderboardDrop.tsxcomponents/waves/leaderboard/drops/WaveLeaderboardDrop.tsxcomponents/waves/leaderboard/drops/WaveLeaderboardDrops.tsxcomponents/waves/leaderboard/leaderboardRendererRegistry.tsxcomponents/waves/memes/MemesArtSubmissionModal.tsxcomponents/waves/memes/submission/MemesArtResubmitAction.tsxcomponents/waves/quorum/QuorumProposalCompactContent.tsxcomponents/waves/quorum/quorumProposalMarkdown.tscomponents/waves/small-leaderboard/DefaultWaveSmallLeaderboardDrop.tsxcomponents/waves/small-leaderboard/QuorumWaveSmallLeaderboardDrop.tsxcomponents/waves/small-leaderboard/WaveSmallLeaderboardDefaultDrop.tsxcomponents/waves/small-leaderboard/WaveSmallLeaderboardDrop.tsxcomponents/waves/small-leaderboard/WaveSmallLeaderboardItemContent.tsxcomponents/waves/small-leaderboard/WaveSmallLeaderboardTopThreeDrop.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- components/waves/quorum/QuorumProposalCompactContent.tsx
- tests/components/waves/quorum/QuorumProposalCompactContent.test.tsx
ragnep
approved these changes
Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests