fix(copilot): dedupe transcript replay blocks#13071
Conversation
Abhi1992002
requested review from
0ubbe and
ntindle
and removed request for
a team
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
abhi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR refactors the copilot message deduplication logic to handle SSE transcript replays. The implementation extracts fingerprint helpers, adds transcript-prefix replay detection, and restructures ChangesMessage Deduplication Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13071 +/- ##
==========================================
- Coverage 70.59% 70.56% -0.04%
==========================================
Files 2193 2193
Lines 164774 164800 +26
Branches 16822 16828 +6
==========================================
- Hits 116325 116290 -35
- Misses 45093 45146 +53
- Partials 3356 3364 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts (1)
358-386: 💤 Low valuePrefix-replay algorithm is correct; a small bound tightening would skip provably useless iterations.
Traced through the documented scenario
[u1, a1, u2, a2, u1', a1', u2', a2']:
- The early return guards
length < MIN_PREFIX_REPLAY_MESSAGES * 2.- For
i ∈ [1, MIN_PREFIX_REPLAY_MESSAGES - 1], the innerwhileis capped byreplayLength < i, soreplayLengthcan never reachMIN_PREFIX_REPLAY_MESSAGESand the outer iteration is a no-op.- At
i = 4, all four fingerprints match and the block 4–7 is dropped;i += replayLength - 1followed byi++correctly advances past the dropped range, so multiple consecutive replays are handled.Optional micro-tightening — start the outer loop at
MIN_PREFIX_REPLAY_MESSAGESto make the no-op iterations impossible by construction, and document thereplayLength < i(no-overlap) invariant inline:♻️ Optional clarity tweak
- for (let i = 1; i < messages.length; i++) { + // Smallest replay starts at i === MIN_PREFIX_REPLAY_MESSAGES because the + // prefix and the replay must each contain at least that many messages and + // must not overlap (replayLength < i below). + for (let i = MIN_PREFIX_REPLAY_MESSAGES; i < messages.length; i++) { if (dropped.has(i)) continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/helpers.ts around lines 358 - 386, The outer loop in removeTranscriptPrefixReplays should start at MIN_PREFIX_REPLAY_MESSAGES to avoid provably useless iterations; update the for loop from for (let i = 1; ...) to begin at i = MIN_PREFIX_REPLAY_MESSAGES and add an inline comment near the while that documents the no-overlap invariant (replayLength < i) to clarify why early indices cannot produce a valid replay match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/helpers.ts:
- Around line 358-386: The outer loop in removeTranscriptPrefixReplays should
start at MIN_PREFIX_REPLAY_MESSAGES to avoid provably useless iterations; update
the for loop from for (let i = 1; ...) to begin at i =
MIN_PREFIX_REPLAY_MESSAGES and add an inline comment near the while that
documents the no-overlap invariant (replayLength < i) to clarify why early
indices cannot produce a valid replay match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6bfdd376-5545-4657-a618-85cfdfc669ca
📒 Files selected for processing (2)
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
📜 Review details
⏰ 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). (7)
- GitHub Check: check API types
- GitHub Check: integration_test
- GitHub Check: Seer Code Review
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (11)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend developmentFormat frontend code using
pnpm format
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Fully capitalize acronyms in symbols, e.g.graphID,useBackendAPI
No linter suppressors (//@ts-ignore``,// eslint-disable) — fix the actual issue
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/generated/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development
autogpt_platform/frontend/**/*.{ts,tsx}: Use function declarations (not arrow functions) for components/handlers
Noanytypes unless the value genuinely can be anything
Keep render functions and hooks under ~50 lines; extract named helpers or sub-components when they grow longer
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Use generated API hooks from@/app/api/__generated__/endpoints/following the patternuse{Method}{Version}{OperationName}, and regenerate withpnpm generate:api
Separate render logic from business logic using component.tsx + useComponent.ts + helpers.ts pattern, colocate state when possible and avoid creating large components, use sub-components in local/componentsfolder
Use function declarations for components and handlers, use arrow functions only for callbacks
Do not useuseCallbackoruseMemounless asked to optimise a given function
autogpt_platform/frontend/src/**/*.{ts,tsx}: Keep files under ~200 lines; extract sub-components or hooks into their own files when a file grows beyond this
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}
Always import the-Icon-suffixed alias from@phosphor-icons/react(e.g.TrashIcon,PlusIcon,SquareIcon) — bare exports are deprecated
Do not useuseCallbackoruseMemounless asked to optimize a given function
Never usesrc/components/__legacy__/*— use design system components fromsrc/components/
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/frontend/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
No barrel files or
index.tsre-exports in the frontend
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/frontend/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not type hook returns, let Typescript infer as much as possible
autogpt_platform/frontend/src/**/*.ts: Extract component logic into custom hooks grouped by concern, not by component, with each hook in its own.tsfile
Do not type hook returns; let TypeScript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never type with
any, if no types available useunknown
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
autogpt_platform/frontend/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/**/*.{test,spec}.{ts,tsx}: Use Vitest + RTL + MSW for integration tests as the primary testing approach (~90%, page-level), use Playwright for E2E critical flows, and use Storybook for design system components
Run frontend integration tests withpnpm test:unit(Vitest + RTL + MSW)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.ts
autogpt_platform/frontend/src/app/**/__tests__/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/AGENTS.md)
Write integration tests in
__tests__/next topage.tsxusing Vitest + RTL + MSW for new pages/features
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.ts
autogpt_platform/frontend/src/**/__tests__/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/AGENTS.md)
Use Orval-generated MSW handlers from
@/app/api/__generated__/endpoints/{tag}/{tag}.msw.tsfor API mocking
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.ts
autogpt_platform/frontend/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/AGENTS.md)
Avoid index and barrel files
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
🧠 Learnings (5)
📚 Learning: 2026-03-24T02:23:31.305Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12526
File: autogpt_platform/frontend/src/app/(platform)/copilot/components/RateLimitResetDialog/RateLimitResetDialog.tsx:0-0
Timestamp: 2026-03-24T02:23:31.305Z
Learning: In the Copilot platform UI code, follow the established Orval hook `onError` error-handling convention: first explicitly detect/handle `ApiError`, then read `error.response?.detail` (if present) as the primary message; if not available, fall back to `error.message`; and finally fall back to a generic string message. This convention should be used for generated Orval hooks even if the custom Orval mutator already maps details into `ApiError.message`, to keep consistency across hooks/components (e.g., `useCronSchedulerDialog.ts`, `useRunGraph.ts`, and rate-limit/reset flows).
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
📚 Learning: 2026-04-01T18:54:16.035Z
Learnt from: Bentlybro
Repo: Significant-Gravitas/AutoGPT PR: 12633
File: autogpt_platform/frontend/src/app/(platform)/library/components/AgentFilterMenu/AgentFilterMenu.tsx:3-10
Timestamp: 2026-04-01T18:54:16.035Z
Learning: In the frontend, the legacy Select component at `@/components/__legacy__/ui/select` is an intentional, codebase-wide visual-consistency pattern. During code reviews, do not flag or block PRs merely for continuing to use this legacy Select. If a migration to the newer design-system Select is desired, bundle it into a single dedicated cleanup/migration PR that updates all Select usages together (e.g., avoid piecemeal replacements).
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
📚 Learning: 2026-04-07T09:24:16.582Z
Learnt from: 0ubbe
Repo: Significant-Gravitas/AutoGPT PR: 12686
File: autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/PainPointsStep.test.tsx:1-19
Timestamp: 2026-04-07T09:24:16.582Z
Learning: In Significant-Gravitas/AutoGPT’s `autogpt_platform/frontend` (Vite + `vitejs/plugin-react` with the automatic JSX transform), do not flag usages of React types/components (e.g., `React.ReactNode`) in `.ts`/`.tsx` files as missing `React` imports. Since the React namespace is made available by the project’s TS/Vite setup, an explicit `import React from 'react'` or `import type { ReactNode } ...` is not required; only treat it as missing if typechecking (e.g., `pnpm types`) would actually fail.
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
📚 Learning: 2026-04-02T05:43:49.128Z
Learnt from: 0ubbe
Repo: Significant-Gravitas/AutoGPT PR: 12640
File: autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/WelcomeStep.tsx:13-13
Timestamp: 2026-04-02T05:43:49.128Z
Learning: Do not flag `import { Question } from "phosphor-icons/react"` as an invalid import. `Question` is a valid named export from `phosphor-icons/react` (as reflected in the package’s generated `.d.ts` files and re-exports via `dist/index.d.ts`), so it should be treated as a supported named export during code reviews.
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts
📚 Learning: 2026-04-20T20:07:22.981Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 11235
File: autogpt_platform/frontend/src/app/(platform)/admin/diagnostics/__tests__/ExecutionsTable.test.tsx:27-76
Timestamp: 2026-04-20T20:07:22.981Z
Learning: In this codebase, Orval-generated API modules under `src/app/api/__generated__/` are not committed to git and must be generated via `pnpm generate:api` (requires a running backend). In integration tests, it’s acceptable—and expected—to stub generated hooks/modules by mocking them with `vi.mock("@/app/api/__generated__/endpoints/{tag}/{tag}")`. Do not treat `vi.mock` of these generated hook modules as a violation of the MSW handler guideline, since the corresponding MSW handlers cannot be imported at test time when generated files are absent.
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.ts
🔇 Additional comments (3)
autogpt_platform/frontend/src/app/(platform)/copilot/helpers.ts (2)
327-346: Fingerprint helpers look correct and well-justified.The JSON-encoded mapping correctly avoids separator-collision false positives, and the
JSON.stringify(p)fallback for parts withouttext/toolCallId(e.g.step-start) prevents structurally-different parts from collapsing into the same empty fingerprint element. The role prefix inmessageReplayFingerprintmakes prefix matching role-aware, which is the right behavior for a transcript replay (a user message with the same text as a prior assistant message must not be considered a match).
398-439: Three-stage pipeline is well-structured.ID dedup → prefix-replay dedup → assistant-content fingerprint scoped by
lastUserMsgIDis a clean ordering: the ID pass collapses exact duplicates first so subsequent fingerprint comparisons aren't quadratic over redundant entries, and scoping the assistant fingerprint by the preceding user-message ID (rather than text) correctly preserves identical answers to repeated identical questions, as exercised by the existing "keeps second answer when same question is asked twice in one session" test.autogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.ts (1)
346-362: Regression test accurately exercises the new prefix-replay path.Differing IDs between the original (
u1..a2) and the replayed block (u1-replay..a2-replay) ensure the test cannot pass via ID-dedup alone and must go throughremoveTranscriptPrefixReplays. The asserted ordering["u1", "a1", "u2", "a2"]also confirms the function preserves the original prefix rather than the replayed one.One optional addition worth considering for follow-up coverage (skip if not desired): a test asserting two consecutive replays (
prefix + replay + replay) collapse to just the prefix, which would lock in thei += replayLength - 1advancement behavior. Not blocking.
Abhi1992002
force-pushed
the
fix/frontend-deduplicate-chat-messages
branch
from
fd8fb27 to
aa3df53
Compare
The inner while is capped by `replayLength < i`, so iterations with i < MIN_PREFIX_REPLAY_MESSAGES can never accumulate enough matches to clear the drop threshold. Starting the outer loop at the minimum replay length skips those provably no-op iterations and makes the algorithm's structure explicit.
Abhi1992002
force-pushed
the
fix/frontend-deduplicate-chat-messages
branch
from
aa3df53 to
1b67cc2
Compare
2 participants
Summary
autogpt_platform/frontend/src/app/(platform)/copilot/helpers.tsand its colocated unit test.Test Plan
pnpm test:unit 'src/app/(platform)/copilot/__tests__/helpers.test.ts'pnpm formatpnpm lintpnpm types