Ensure note save payloads fail visibly#228
Merged
Merged
Conversation
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds thought-normalization utilities and tests; updates two API endpoints to validate/reject invalid thought submissions (including duplicate completion notes), verifies DB insert counts via Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Endpoint
participant Norm as Thought Validation
participant DB as Database
Client->>API: POST /batch or /record-personal-session (body.thoughts)
API->>Norm: normalizeThoughtInputs(body.thoughts)
Norm-->>API: ThoughtNormalizationResult (submittedCount, invalidCount, thoughts)
API->>API: hasRejectedSubmittedThoughts(result)?
alt Rejected or invalid shape
API-->>Client: 400 Invalid thoughts payload
else Valid
API->>API: check duplicate completion notes?
alt Duplicate completion notes found
API-->>Client: 400 Duplicate completion note
else
API->>DB: INSERT thoughts ... RETURNING(id)
DB-->>API: inserted rows
API->>API: verify inserted count == thoughts.length
alt Mismatch
API-->>API: throw THOUGHT_INSERT_MISMATCH
else
API-->>Client: 200 OK with inserted thoughts
end
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/thoughtSaving.test.ts (1)
8-41: Add a mixed valid/invalid payload regression.The suite only covers all-valid and all-invalid arrays. The PR’s main behavior change is that even one bad note in an otherwise valid submission must fail visibly, so please lock that case in too.
Suggested test
describe("normalizeThoughtInputs", () => { + it("rejects mixed valid and invalid submitted payloads", () => { + const result = normalizeThoughtInputs([ + { timeInSession: 12, text: "kept if normalized alone" }, + { timeInSession: "13", text: "invalid" }, + ]); + + assert.equal(result.submittedCount, 2); + assert.equal(result.invalidCount, 1); + assert.deepEqual(result.thoughts, [{ timeInSession: 12, text: "kept if normalized alone" }]); + assert.equal(hasRejectedSubmittedThoughts(result), true); + }); + it("accepts web completion note payloads", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/thoughtSaving.test.ts` around lines 8 - 41, Add a new test case to cover a mixed valid/invalid payload for normalizeThoughtInputs: call normalizeThoughtInputs with an array containing at least one valid thought (proper timeInSession number and non-empty text) and one invalid thought (e.g., timeInSession as string or empty text), then assert submittedCount equals total inputs, invalidCount equals number of invalid items, thoughts contains only the valid normalized entries, and hasRejectedSubmittedThoughts(result) is true; reference normalizeThoughtInputs and hasRejectedSubmittedThoughts to locate where to add this test in the existing describe block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/buddy/sessions/`[id]/record-personal-session/route.ts:
- Around line 126-136: The code currently calls
normalizeThoughtInputs(body.thoughts) which masks non-array inputs and lets
malformed payloads silently pass; instead, validate that body.thoughts is an
array (and not null/undefined) before calling normalizeThoughtInputs and return
a 400 error if it’s not an array or is otherwise malformed; update the handler
in route.ts to check Array.isArray(body.thoughts) (and optionally verify each
item is an object/has required fields) and respond with NextResponse.json({
error: "Invalid thoughts payload" }, { status: 400 }) prior to invoking
normalizeThoughtInputs so hasRejectedSubmittedThoughts and thoughtItems only
operate on proper arrays.
In `@src/app/api/thoughts/batch/route.ts`:
- Around line 46-56: The route is silently dropping duplicate completion notes
(timeInSession === -1) because normalizeThoughtInputs collapses them before the
new count check; update the validation to detect multiple completion notes
before collapsing: in route.ts, after computing normalizedResult (from
normalizeThoughtInputs) or by inspecting the original thoughtItems, count items
with timeInSession === -1 and if count > 1 return a 400 error (similar to the
existing invalid payload response). Reference normalizeThoughtInputs,
hasRejectedSubmittedThoughts and the variable normalizedResult/normalized to
locate where to add this pre-collapse duplicate-completion validation.
---
Nitpick comments:
In `@src/lib/thoughtSaving.test.ts`:
- Around line 8-41: Add a new test case to cover a mixed valid/invalid payload
for normalizeThoughtInputs: call normalizeThoughtInputs with an array containing
at least one valid thought (proper timeInSession number and non-empty text) and
one invalid thought (e.g., timeInSession as string or empty text), then assert
submittedCount equals total inputs, invalidCount equals number of invalid items,
thoughts contains only the valid normalized entries, and
hasRejectedSubmittedThoughts(result) is true; reference normalizeThoughtInputs
and hasRejectedSubmittedThoughts to locate where to add this test in the
existing describe block.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5db5ba37-5737-421e-a49f-971e934c335c
📒 Files selected for processing (5)
ios/StillPointShared/Tests/StillPointSharedTests/RecordBuddyPersonalSessionRequestEncodingTests.swiftsrc/app/api/buddy/sessions/[id]/record-personal-session/route.tssrc/app/api/thoughts/batch/route.tssrc/lib/thoughtSaving.test.tssrc/lib/thoughtSaving.ts
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/thoughtSaving.test.ts (1)
8-55: Tests cover core normalization scenarios well.The test cases effectively validate the main behaviors: filtering invalid entries, whitespace trimming, handling multiple valid notes, and rejection semantics.
Consider adding edge case coverage for:
- Empty array input (
normalizeThoughtInputs([]))- Text that becomes empty after trimming (
{ timeInSession: 1, text: " " })- Boundary
timeInSessionvalues (0, negative values other than -1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/thoughtSaving.test.ts` around lines 8 - 55, Add unit tests to cover edge cases for normalizeThoughtInputs: add a test for an empty array input (normalizeThoughtInputs([])) asserting submittedCount 0, invalidCount 0, thoughts [], and hasRejectedSubmittedThoughts(...) false; add a test where text trims to empty (e.g., { timeInSession: 1, text: " " }) asserting it is treated as invalid and results in invalidCount increment and no saved thoughts; and add tests for boundary timeInSession values (0 and negative values other than -1) to assert they are either accepted or rejected per current intended behavior, referencing normalizeThoughtInputs and hasRejectedSubmittedThoughts to locate where to add these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/thoughtSaving.test.ts`:
- Around line 8-55: Add unit tests to cover edge cases for
normalizeThoughtInputs: add a test for an empty array input
(normalizeThoughtInputs([])) asserting submittedCount 0, invalidCount 0,
thoughts [], and hasRejectedSubmittedThoughts(...) false; add a test where text
trims to empty (e.g., { timeInSession: 1, text: " " }) asserting it is treated
as invalid and results in invalidCount increment and no saved thoughts; and add
tests for boundary timeInSession values (0 and negative values other than -1) to
assert they are either accepted or rejected per current intended behavior,
referencing normalizeThoughtInputs and hasRejectedSubmittedThoughts to locate
where to add these assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 12292e9d-b115-41db-857c-d740dc269b39
📒 Files selected for processing (3)
src/app/api/buddy/sessions/[id]/record-personal-session/route.tssrc/app/api/thoughts/batch/route.tssrc/lib/thoughtSaving.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/api/buddy/sessions/[id]/record-personal-session/route.ts
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
This was referenced Apr 27, 2026
9 tasks
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
400 Invalid thoughts payloadwhen submitted note payloads are malformed, mixed valid/invalid, non-array in buddy saves, or duplicate completion-note submissions.Testing
node --import tsx --test src/lib/thoughtSaving.test.tsnpm run buildnpx tsc --noEmitswift test(Swift toolchain is not installed in this Linux image)coderabbit review --prompt-only(CodeRabbit CLI is not installed in this image)npx coderabbit review --prompt-only(npm could not resolve a CodeRabbit executable)@cursor reviewcomment attempt failed via GitHub API withResource not accessible by integration; Cursor Bugbot nevertheless started after the PR was marked readySummary by CodeRabbit
Bug Fixes
Tests