Conversation
|
Cursor Agent can help with this pull request. Just |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplace direct trimming with a new formatter Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
5663c46 to
c6a5abb
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/utils/__tests__/call-kent-text-to-speech-notes.test.ts (1)
7-28: Tests are correct and will pass — missing coverage for the empty-input branchAll three test cases correctly exercise the implementation. However, the function's early-exit branch —
if (!cleaned) return cleaned— is left untested. An empty string and a whitespace-only string are both valid inputs worth asserting on.✅ Suggested additional test cases
+test('returns empty string for whitespace-only input', () => { + expect(formatCallKentTextToSpeechNotes(' ')).toBe('') +}) + +test('returns empty string for empty input', () => { + expect(formatCallKentTextToSpeechNotes('')).toBe('') +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/utils/__tests__/call-kent-text-to-speech-notes.test.ts` around lines 7 - 28, Add tests covering the early-exit branch in formatCallKentTextToSpeechNotes: call formatCallKentTextToSpeechNotes('') and formatCallKentTextToSpeechNotes(' ') and assert they return an empty string (i.e., the cleaned value), which exercises the if (!cleaned) return cleaned branch in the function; add these two assertions to the existing test file alongside the other cases so the empty/whitespace inputs are explicitly covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/utils/__tests__/call-kent-text-to-speech-notes.test.ts`:
- Around line 7-28: Add tests covering the early-exit branch in
formatCallKentTextToSpeechNotes: call formatCallKentTextToSpeechNotes('') and
formatCallKentTextToSpeechNotes(' ') and assert they return an empty string
(i.e., the cleaned value), which exercises the if (!cleaned) return cleaned
branch in the function; add these two assertions to the existing test file
alongside the other cases so the empty/whitespace inputs are explicitly covered.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/utils/__tests__/call-kent-text-to-speech-notes.test.tsapp/utils/call-kent-text-to-speech.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/utils/call-kent-text-to-speech.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/semantic-search-test-cache.ts`:
- Around line 4-19: Add a reset helper that clears module-global state to avoid
cross-test leakage: implement a function (e.g., resetTestCache) that calls
memory.clear() and cachifiedSpy.mockReset() (or vi.resetAllMocks for the spy),
export it alongside memory, testCache, and cachifiedSpy, and call that helper
from a test lifecycle hook (e.g., beforeEach(() => resetTestCache())) so the Map
`memory`, the cache wrapper `testCache`, and the spy `cachifiedSpy` are reset
between tests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/utils/__tests__/call-kent-text-to-speech-notes.test.tstests/semantic-search-test-cache.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/utils/tests/call-kent-text-to-speech-notes.test.ts
tests/semantic-search-test-cache.ts
Outdated
| export const memory = new Map<string, unknown>() | ||
|
|
||
| export const testCache = { | ||
| name: 'semantic-search-test-cache', | ||
| get(key: string) { | ||
| return (memory.get(key) as any) ?? null | ||
| }, | ||
| async set(key: string, entry: unknown) { | ||
| memory.set(key, entry) | ||
| }, | ||
| async delete(key: string) { | ||
| memory.delete(key) | ||
| }, | ||
| } | ||
|
|
||
| export const cachifiedSpy = vi.fn(baseCachified as any) |
There was a problem hiding this comment.
Add a reset hook to prevent cross-test cache leakage.
memory and cachifiedSpy are module‑global and never cleared, so tests can become order‑dependent. Consider adding a reset helper plus a beforeEach to ensure isolation.
♻️ Suggested fix
-import { vi } from 'vitest'
+import { beforeEach, vi } from 'vitest'
@@
export const cachifiedSpy = vi.fn(baseCachified as any)
+
+export const resetTestCache = () => {
+ memory.clear()
+ cachifiedSpy.mockClear()
+}
+
+beforeEach(() => {
+ resetTestCache()
+})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/semantic-search-test-cache.ts` around lines 4 - 19, Add a reset helper
that clears module-global state to avoid cross-test leakage: implement a
function (e.g., resetTestCache) that calls memory.clear() and
cachifiedSpy.mockReset() (or vi.resetAllMocks for the spy), export it alongside
memory, testCache, and cachifiedSpy, and call that helper from a test lifecycle
hook (e.g., beforeEach(() => resetTestCache())) so the Map `memory`, the cache
wrapper `testCache`, and the spy `cachifiedSpy` are reset between tests.
7d017c4 to
e7b8987
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/utils/__tests__/call-kent-text-to-speech-notes.test.ts (1)
22-28: Consider adding a case-insensitive prefix de-dup test.The formatter explicitly does a case-insensitive comparison (
cleaned.toLowerCase().startsWith(prefixLower)), but no test exercises that path. A user pasting the prefix in a different case (e.g., all-caps) would still trigger de-duplication.💡 Suggested additional test
+test('removes duplicate AI disclosure regardless of casing', () => { + const question = `${AI_VOICE_DISCLOSURE_PREFIX.toUpperCase()} Hello from a typed call.` + const notes = formatCallKentTextToSpeechNotes(question) + expect(notes).toBe( + `${AI_VOICE_DISCLOSURE_PREFIX}\nTyped question: Hello from a typed call.`, + ) +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/utils/__tests__/call-kent-text-to-speech-notes.test.ts` around lines 22 - 28, Add a new unit test that verifies formatCallKentTextToSpeechNotes removes the AI_VOICE_DISCLOSURE_PREFIX regardless of casing: create a question string that uses a different case variant of AI_VOICE_DISCLOSURE_PREFIX (e.g., upper- or mixed-case) followed by a typed question and assert the output matches the expected deduplicated form (prefix on its own line and "Typed question: ..." text); reference AI_VOICE_DISCLOSURE_PREFIX and formatCallKentTextToSpeechNotes so the test targets the case-insensitive startsWith path in that function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/utils/__tests__/call-kent-text-to-speech-notes.test.ts`:
- Around line 22-28: Add a new unit test that verifies
formatCallKentTextToSpeechNotes removes the AI_VOICE_DISCLOSURE_PREFIX
regardless of casing: create a question string that uses a different case
variant of AI_VOICE_DISCLOSURE_PREFIX (e.g., upper- or mixed-case) followed by a
typed question and assert the output matches the expected deduplicated form
(prefix on its own line and "Typed question: ..." text); reference
AI_VOICE_DISCLOSURE_PREFIX and formatCallKentTextToSpeechNotes so the test
targets the case-insensitive startsWith path in that function.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/routes/calls_.record/new.tsxapp/utils/__tests__/call-kent-text-to-speech-notes.test.tsapp/utils/__tests__/semantic-search-cache.server.test.tsapp/utils/__tests__/semantic-search.server.test.tsapp/utils/call-kent-text-to-speech.tsdocs/agents/project-context.mdtests/semantic-search-test-cache.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/utils/call-kent-text-to-speech.ts
- tests/semantic-search-test-cache.ts
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
25ac7fa to
106863a
Compare
Add an AI disclosure prefix to notes for Call Kent podcast recordings generated from typed text.
Note
Low Risk
Small, isolated formatting change for prefilled call notes plus unit tests and docs; no auth, persistence, or core request handling changes.
Overview
Typed-question (text-to-speech) call submissions now prefill
noteswith a standardized format that prepends the AI voice disclosure and labels the content asTyped question:while preserving internal newlines.This introduces
formatCallKentTextToSpeechNotes(with de-dupe/empty handling) and adds unit coverage for trimming, newline preservation, duplicate-prefix stripping, and blank input. Docs also note that cloud/headless Chrome may need mic/camera permissions to avoid/calls/record/newhitting the ErrorBoundary.Written by Cursor Bugbot for commit 106863a. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Tests
Documentation