Add AI-generated flashcard study sets - #782
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Important Review skippedToo many files! This PR contains 117 files, which is 17 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (117)
You can disable this status message by setting the 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 |
|
React Doctor found 4 new issues in 3 files · 4 warnings · score 83 / 100 (Needs work) · 2 fixed · vs 4 warnings
Reviewed by React Doctor for commit |
![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: 9fdd913a64
ℹ️ 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".
| const rate = (rating: FlashcardStudyRating) => { | ||
| if (settling) return; | ||
| recordRating.mutate({ cardId: currentCard.id, rating }); | ||
| setSession((current) => ({ | ||
| ...current, | ||
| ratingFeedback: rating, | ||
| })); |
There was a problem hiding this comment.
When the rating request fails because of a network or server error, this code has already started the success animation, whose completion advances to the next card regardless of the mutation outcome. The mutation's onError only restores the cached study state, so the failed card remains unreviewed while the session silently moves past it; advance only after persistence succeeds, or return to the failed card on error.
Useful? React with 👍 / 👎.
| if (!canSend || inputStatus !== "ready") return; | ||
| const text = takeDirectPrompt(threadId, directPrompt.id); | ||
| if (text) queueMicrotask(() => sendDirectPrompt(text)); |
There was a problem hiding this comment.
When an AI action is clicked while the current turn is still streaming, this effect waits for canSend and later calls sendMessage, which builds its workspace context at that later moment. The flashcard Hint/Explain prompts only say “current flashcard,” so if the user navigates to another card or item before the prior turn finishes, the queued action is sent against the new view and can answer for the wrong card; preserve the click-time context with the queued prompt or make the prompt identify the card explicitly.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 87 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/flashcards/FlashcardViewer.tsx">
<violation number="1" location="src/features/workspaces/components/flashcards/FlashcardViewer.tsx:536">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The Hint and Explain buttons send generic prompts with no card context, so the AI cannot know which flashcard is active. Include the current card content in the prompt sent to `sendComposerPrompt`, similar to how other callers embed workspace context.
For example, the Hint action can pass the card front text: `sendComposerPrompt(item.workspaceId, `Flashcard front: "${extractText(currentCard.front)}"\n\nGive me a helpful hint without revealing the answer.`)`</violation>
</file>
<file name="src/features/workspaces/export/workspace-export-archive.ts">
<violation number="1" location="src/features/workspaces/export/workspace-export-archive.ts:124">
P3: serializeFlashcardsToMarkdown interpolates the user-controlled item.name directly into a `# ` markdown heading without escaping. A set name containing a newline or markdown characters (e.g. `#`, `-`) breaks the generated file structure, producing multiple headings or injected formatting lines that split the cards apart. The document export path doesn't embed the name at all, so this is new surface. Escape the name (e.g. replace newlines and escape leading `#`) or emit it as plain text rather than a heading.</violation>
</file>
<file name="src/features/workspaces/model/workspace-item-create-bootstrap.ts">
<violation number="1" location="src/features/workspaces/model/workspace-item-create-bootstrap.ts:24">
P1: When a flashcard is created without `initialContent`, this fallback returns an empty string and persistence rejects the structured item. The accepted create schema and generic New Flashcards flow allow that request, so flashcard creation fails instead of creating an item; require a valid flashcard payload or prevent empty flashcard creation before this path.</violation>
</file>
<file name="src/features/workspaces/contracts.ts">
<violation number="1" location="src/features/workspaces/contracts.ts:385">
P3: The relaxed validation now makes `initialContent` acceptable for every type, but `getCreateWorkspaceItemInitialContent` only honors it for `document`: the `flashcard` branch builds content from `input.cards` and ignores `initialContent`, and `folder`/`file` silently drop it. A `createWorkspaceItemFn` call for type `flashcard` passing `initialContent` (and no `cards`, which is absent from `createWorkspaceItemInputSchema`) reaches `createFlashcardSetFromHtml(undefined)` and fails, despite the schema accepting the payload. Keep the check aligned with the type that actually consumes the field, and update it to also drop the misleading error message ("only be provided for documents") or handle the flashcard `initialContent` explicitly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ? input.initialContent?.trim() | ||
| ? input.initialContent | ||
| : stringifyTiptapDocumentJson(createInitialTiptapDocumentJson()) | ||
| : (input.initialContent ?? ""); |
There was a problem hiding this comment.
P1: When a flashcard is created without initialContent, this fallback returns an empty string and persistence rejects the structured item. The accepted create schema and generic New Flashcards flow allow that request, so flashcard creation fails instead of creating an item; require a valid flashcard payload or prevent empty flashcard creation before this path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/model/workspace-item-create-bootstrap.ts, line 24:
<comment>When a flashcard is created without `initialContent`, this fallback returns an empty string and persistence rejects the structured item. The accepted create schema and generic New Flashcards flow allow that request, so flashcard creation fails instead of creating an item; require a valid flashcard payload or prevent empty flashcard creation before this path.</comment>
<file context>
@@ -0,0 +1,31 @@
+ ? input.initialContent?.trim()
+ ? input.initialContent
+ : stringifyTiptapDocumentJson(createInitialTiptapDocumentJson())
+ : (input.initialContent ?? "");
+ const metadataJson =
+ contentKind === "document"
</file context>
| label="Front" | ||
| content={currentCard.front} | ||
| action={ | ||
| <FlashcardAiAction |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
The Hint and Explain buttons send generic prompts with no card context, so the AI cannot know which flashcard is active. Include the current card content in the prompt sent to sendComposerPrompt, similar to how other callers embed workspace context.
For example, the Hint action can pass the card front text: sendComposerPrompt(item.workspaceId, Flashcard front: "${extractText(currentCard.front)}"\n\nGive me a helpful hint without revealing the answer.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/flashcards/FlashcardViewer.tsx, line 536:
<comment>The Hint and Explain buttons send generic prompts with no card context, so the AI cannot know which flashcard is active. Include the current card content in the prompt sent to `sendComposerPrompt`, similar to how other callers embed workspace context.
For example, the Hint action can pass the card front text: `sendComposerPrompt(item.workspaceId, `Flashcard front: "${extractText(currentCard.front)}"\n\nGive me a helpful hint without revealing the answer.`)`</comment>
<file context>
@@ -0,0 +1,851 @@
+ label="Front"
+ content={currentCard.front}
+ action={
+ <FlashcardAiAction
+ label="Hint"
+ onSend={() =>
</file context>
| return archivePaths; | ||
| } | ||
|
|
||
| function serializeFlashcardsToMarkdown(item: WorkspaceItem, set: FlashcardSetContent) { |
There was a problem hiding this comment.
P3: serializeFlashcardsToMarkdown interpolates the user-controlled item.name directly into a # markdown heading without escaping. A set name containing a newline or markdown characters (e.g. #, -) breaks the generated file structure, producing multiple headings or injected formatting lines that split the cards apart. The document export path doesn't embed the name at all, so this is new surface. Escape the name (e.g. replace newlines and escape leading #) or emit it as plain text rather than a heading.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/export/workspace-export-archive.ts, line 124:
<comment>serializeFlashcardsToMarkdown interpolates the user-controlled item.name directly into a `# ` markdown heading without escaping. A set name containing a newline or markdown characters (e.g. `#`, `-`) breaks the generated file structure, producing multiple headings or injected formatting lines that split the cards apart. The document export path doesn't embed the name at all, so this is new surface. Escape the name (e.g. replace newlines and escape leading `#`) or emit it as plain text rather than a heading.</comment>
<file context>
@@ -114,6 +121,15 @@ function buildArchivePathIndex(
return archivePaths;
}
+function serializeFlashcardsToMarkdown(item: WorkspaceItem, set: FlashcardSetContent) {
+ const cards = set.cards.map((card, index) => {
+ const front = serializeTiptapDocumentToMarkdown(card.front);
</file context>
| input.initialContent !== undefined && | ||
| getWorkspaceItemContentKind(input.type) !== "document" | ||
| ) { | ||
| if (input.initialContent !== undefined && input.type !== "document") { |
There was a problem hiding this comment.
P3: The relaxed validation now makes initialContent acceptable for every type, but getCreateWorkspaceItemInitialContent only honors it for document: the flashcard branch builds content from input.cards and ignores initialContent, and folder/file silently drop it. A createWorkspaceItemFn call for type flashcard passing initialContent (and no cards, which is absent from createWorkspaceItemInputSchema) reaches createFlashcardSetFromHtml(undefined) and fails, despite the schema accepting the payload. Keep the check aligned with the type that actually consumes the field, and update it to also drop the misleading error message ("only be provided for documents") or handle the flashcard initialContent explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/contracts.ts, line 385:
<comment>The relaxed validation now makes `initialContent` acceptable for every type, but `getCreateWorkspaceItemInitialContent` only honors it for `document`: the `flashcard` branch builds content from `input.cards` and ignores `initialContent`, and `folder`/`file` silently drop it. A `createWorkspaceItemFn` call for type `flashcard` passing `initialContent` (and no `cards`, which is absent from `createWorkspaceItemInputSchema`) reaches `createFlashcardSetFromHtml(undefined)` and fails, despite the schema accepting the payload. Keep the check aligned with the type that actually consumes the field, and update it to also drop the misleading error message ("only be provided for documents") or handle the flashcard `initialContent` explicitly.</comment>
<file context>
@@ -382,10 +382,7 @@ export const createWorkspaceItemInputSchema = z
- input.initialContent !== undefined &&
- getWorkspaceItemContentKind(input.type) !== "document"
- ) {
+ if (input.initialContent !== undefined && input.type !== "document") {
context.addIssue({
code: "custom",
</file context>
Greptile SummaryThis change adds flashcard workspace items with AI creation and editing, rich content reads and exports, per-user study progress, and a study viewer with ratings, modes, shuffling, hints, and explanations. It also updates resumed chat streaming to retain continuation state. Flashcard continuation was exercised successfully: reads returned each card once, rejected stale and malformed cursors, and detected changed content. The rendered study view was also exercised through final-card completion and a simulated failed optimistic save; both flows recovered to an enabled, usable state without duplicate rating submissions. One issue remains: a batch of flashcard reads can exceed the configured shared response-size budget after per-card reference metadata is added. T-Rex validation blockedConcurrent first-time study-rating persistence could not be exercised because the required PostgreSQL service is unavailable. The repository's local database setup did not complete, connections to localhost port 5432 were refused, and the environment lacks Confidence Score: 4/5The change is not ready to merge until flashcard read budgeting accounts for the final referenced response size. The response-size failure was reproduced with a runtime check that exercised the real read and reference-annotation path. Study-view completion, optimistic rollback, and cursor behavior were exercised successfully. Database concurrency could not be run without PostgreSQL. Files Needing Attention: src/features/workspaces/content/workspace-content-reader.ts needs the budget calculation corrected so it measures or reserves the final serialized flashcard response, including reference metadata.
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "fix(chat): preserve continuation state o..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const contentBytes = encoder.encode( | ||
| "content" in read | ||
| ? read.content | ||
| : JSON.stringify( | ||
| read.cards.map((card) => ({ | ||
| ...card, | ||
| backReference: "wr_00000000", | ||
| frontReference: "wr_00000000", | ||
| })), | ||
| ), | ||
| ).byteLength; |
There was a problem hiding this comment.
The batch limit is calculated from the raw flashcard payload with placeholder references, but the returned tool result later adds durable per-side references and serialization overhead. A boundary batch admitted at 2,162,647 bytes produced a 2,166,981-byte annotated result—4,293 bytes above maxWorkspaceContentBatchBytes. Account for the final serialized read result, including references, or reserve sufficient overhead before accepting another flashcard response.
Artifacts
Authored flashcard content-contract runtime check
- This authored Vitest check invokes readWorkspaceContent and reference annotation across continuation, mutation, invalid-cursor, and batch-budget boundary cases, exposing the post-annotation size overage.
Flashcard content-contract runtime output
- This executed Vitest output shows all continuation and cursor assertions passed while the annotated batch output exceeded the shared budget by 4,293 bytes, confirming the response-size contract violation.
There was a problem hiding this comment.
9 issues found across 65 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/flashcards/flashcard-edits.ts">
<violation number="1" location="src/features/workspaces/flashcards/flashcard-edits.ts:126">
P2: When `replace_text` grows a side beyond 8,000 characters, this path stores it because it validates only each edit's replacement, not the resulting HTML. Validate `replaced.text` with `flashcardSideHtmlSchema` before parsing, matching `update` and `replace` limits.</violation>
<violation number="2" location="src/features/workspaces/flashcards/flashcard-edits.ts:144">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
`card` is always defined here because `cardIndex` was already validated by `findCardIndex` (which throws `ref_not_found` or `ref_stale` on invalid refs), so `splice(cardIndex, 1)` must return the element. Remove this unreachable defensive branch.</violation>
</file>
<file name="src/features/workspaces/ai/ai-thread.ts">
<violation number="1" location="src/features/workspaces/ai/ai-thread.ts:576">
P2: When a turn reads many document blocks or flashcards, this retains every active read ref even though reconciliation caps cited refs at 50. The resulting hidden message part can grow with every paginated read and inflate persisted transcripts and subsequent chat context. Bound retained refs or separate the Code Mode refs that require retention from ordinary read refs.</violation>
</file>
<file name="src/features/workspaces/locations/workspace-location.ts">
<violation number="1" location="src/features/workspaces/locations/workspace-location.ts:141">
P2: When one batch reads the same card or document block across a concurrent content change, this deduplication keeps the first revision while both results receive its ref. Preserve distinct location revisions or reject conflicting duplicates before producing model output, so edits do not unexpectedly return `ref_stale`.</violation>
</file>
<file name="src/features/workspaces/ai/workspace-tool-result-adapters.ts">
<violation number="1" location="src/features/workspaces/ai/workspace-tool-result-adapters.ts:22">
P1: When a persisted workspace tool result uses an older schema, `projectOutput` now throws instead of forwarding it, so chats containing that history cannot resume. Restore the `safeParse` fallback for replayed results, or migrate all persisted tool outputs before enabling this strict parse.</violation>
</file>
<file name="src/features/workspaces/documents/document-ai-edits.ts">
<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:235">
P2: When two blocks have identical visible content, `move` changes only their stable block IDs, then `documentsHaveSameVisibleContent` strips those IDs and returns `no_change`, so the move is never persisted. Preserve block-ID order when checking a move, or bypass this visible-content check for moves.</violation>
</file>
<file name="src/features/workspaces/ai/workspace-references.ts">
<violation number="1" location="src/features/workspaces/ai/workspace-references.ts:15">
P1: When users reopen or continue threads created before this rename, `collectWorkspaceReferenceRecords` drops their persisted citations because it only parses `data-workspace-references`. Support the legacy part type during rollout or migrate existing message parts before switching identifiers.</violation>
</file>
<file name="src/features/workspaces/components/flashcards/FlashcardViewer.tsx">
<violation number="1" location="src/features/workspaces/components/flashcards/FlashcardViewer.tsx:316">
P2: When a rating request outlives its animation, the user can rate a later card before this callback runs. The callback then jumps the session back to the failed card, disrupting the later review; only restore the failed card when it is still the pending session state, or track pending ratings separately.</violation>
</file>
<file name="patches/agents@0.19.0.patch">
<violation number="1" location="patches/agents@0.19.0.patch:60">
P2: When a recovered tool input has the same keys in a different insertion order, this comparison fails even though the inputs are equivalent. The assistant then keeps a new ID and can be persisted as a duplicate row; compare inputs structurally or with canonicalized keys.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| projectOutput: (output: unknown) => { | ||
| const parsed = input.outputSchema.safeParse(output); | ||
| return (parsed.success ? input.projectOutput(parsed.data) : output) as JSONValue; | ||
| return input.projectOutput(input.outputSchema.parse(output)) as JSONValue; |
There was a problem hiding this comment.
P1: When a persisted workspace tool result uses an older schema, projectOutput now throws instead of forwarding it, so chats containing that history cannot resume. Restore the safeParse fallback for replayed results, or migrate all persisted tool outputs before enabling this strict parse.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/workspace-tool-result-adapters.ts, line 22:
<comment>When a persisted workspace tool result uses an older schema, `projectOutput` now throws instead of forwarding it, so chats containing that history cannot resume. Restore the `safeParse` fallback for replayed results, or migrate all persisted tool outputs before enabling this strict parse.</comment>
<file context>
@@ -18,12 +18,8 @@ function defineWorkspaceToolResultAdapter<TSchema extends z.ZodTypeAny>(input: {
projectOutput: (output: unknown) => {
- const parsed = input.outputSchema.safeParse(output);
- return (parsed.success ? input.projectOutput(parsed.data) : output) as JSONValue;
+ return input.projectOutput(input.outputSchema.parse(output)) as JSONValue;
},
};
</file context>
| return input.projectOutput(input.outputSchema.parse(output)) as JSONValue; | |
| const parsed = input.outputSchema.safeParse(output); | |
| return (parsed.success ? input.projectOutput(parsed.data) : output) as JSONValue; |
| } from "#/features/workspaces/locations/workspace-location"; | ||
|
|
||
| export const WORKSPACE_CITATIONS_DATA_PART_TYPE = "data-workspace-citations"; | ||
| export const WORKSPACE_REFERENCES_DATA_PART_TYPE = "data-workspace-references"; |
There was a problem hiding this comment.
P1: When users reopen or continue threads created before this rename, collectWorkspaceReferenceRecords drops their persisted citations because it only parses data-workspace-references. Support the legacy part type during rollout or migrate existing message parts before switching identifiers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/workspace-references.ts, line 15:
<comment>When users reopen or continue threads created before this rename, `collectWorkspaceReferenceRecords` drops their persisted citations because it only parses `data-workspace-references`. Support the legacy part type during rollout or migrate existing message parts before switching identifiers.</comment>
<file context>
@@ -4,50 +4,57 @@ import { z } from "zod";
} from "#/features/workspaces/locations/workspace-location";
-export const WORKSPACE_CITATIONS_DATA_PART_TYPE = "data-workspace-citations";
+export const WORKSPACE_REFERENCES_DATA_PART_TYPE = "data-workspace-references";
const MAX_WORKSPACE_CITATIONS_PER_MESSAGE = 50;
const workspaceCitationTagPattern =
</file context>
|
|
||
| for (const location of locations) { | ||
| for (const target of targets) { | ||
| const { location, revision } = |
There was a problem hiding this comment.
P2: When one batch reads the same card or document block across a concurrent content change, this deduplication keeps the first revision while both results receive its ref. Preserve distinct location revisions or reject conflicting duplicates before producing model output, so edits do not unexpectedly return ref_stale.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/locations/workspace-location.ts, line 141:
<comment>When one batch reads the same card or document block across a concurrent content change, this deduplication keeps the first revision while both results receive its ref. Preserve distinct location revisions or reject conflicting duplicates before producing model output, so edits do not unexpectedly return `ref_stale`.</comment>
<file context>
@@ -108,7 +137,9 @@ export function createWorkspaceReferenceRecords(
- for (const location of locations) {
+ for (const target of targets) {
+ const { location, revision } =
+ "location" in target ? target : { location: target, revision: undefined };
const locationKey = getWorkspaceLocationKey(location);
</file context>
| const children = getDocumentChildren(document); | ||
| if (edit.op === "delete") { | ||
| children.splice(targetIndex, 1); | ||
| } else if (edit.op === "move") { |
There was a problem hiding this comment.
P2: When two blocks have identical visible content, move changes only their stable block IDs, then documentsHaveSameVisibleContent strips those IDs and returns no_change, so the move is never persisted. Preserve block-ID order when checking a move, or bypass this visible-content check for moves.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-edits.ts, line 235:
<comment>When two blocks have identical visible content, `move` changes only their stable block IDs, then `documentsHaveSameVisibleContent` strips those IDs and returns `no_change`, so the move is never persisted. Preserve block-ID order when checking a move, or bypass this visible-content check for moves.</comment>
<file context>
@@ -213,35 +218,34 @@ function countDocumentLines(document: TiptapDocumentJson) {
const children = getDocumentChildren(document);
if (edit.op === "delete") {
children.splice(targetIndex, 1);
+ } else if (edit.op === "move") {
+ if (!destinationBlockId || destinationBlockId === blockId) {
+ return { code: "no_change", status: "failed" };
</file context>
| const reconciled = reconcileWorkspaceMessageReferences( | ||
| message, | ||
| [...transcriptReferences, ...this.activeWorkspaceReferences], | ||
| this.activeWorkspaceReferences, |
There was a problem hiding this comment.
P2: When a turn reads many document blocks or flashcards, this retains every active read ref even though reconciliation caps cited refs at 50. The resulting hidden message part can grow with every paginated read and inflate persisted transcripts and subsequent chat context. Bound retained refs or separate the Code Mode refs that require retention from ordinary read refs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread.ts, line 576:
<comment>When a turn reads many document blocks or flashcards, this retains every active read ref even though reconciliation caps cited refs at 50. The resulting hidden message part can grow with every paginated read and inflate persisted transcripts and subsequent chat context. Bound retained refs or separate the Code Mode refs that require retention from ordinary read refs.</comment>
<file context>
@@ -566,13 +567,14 @@ export function createAIThreadClass(getUserAIStore: () => typeof UserAIStore) {
+ const reconciled = reconcileWorkspaceMessageReferences(
+ message,
+ [...transcriptReferences, ...this.activeWorkspaceReferences],
+ this.activeWorkspaceReferences,
+ );
</file context>
| recordRating.mutate( | ||
| { cardId, rating }, | ||
| { | ||
| onError: () => |
There was a problem hiding this comment.
P2: When a rating request outlives its animation, the user can rate a later card before this callback runs. The callback then jumps the session back to the failed card, disrupting the later review; only restore the failed card when it is still the pending session state, or track pending ratings separately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/flashcards/FlashcardViewer.tsx, line 316:
<comment>When a rating request outlives its animation, the user can rate a later card before this callback runs. The callback then jumps the session back to the failed card, disrupting the later review; only restore the failed card when it is still the pending session state, or track pending ratings separately.</comment>
<file context>
@@ -302,7 +309,22 @@ function FlashcardStudySession({
+ recordRating.mutate(
+ { cardId, rating },
+ {
+ onError: () =>
+ setSession((current) => {
+ const failedIndex = current.cardIds.indexOf(cardId);
</file context>
| + id: serverMessage.id | ||
| + }; | ||
| + } | ||
| + if (serverMessage.role === "assistant" && incomingToolCalls.some((incomingPart) => serverMessage.parts.some((serverPart) => "toolCallId" in serverPart && serverPart.toolCallId === incomingPart.toolCallId && JSON.stringify(serverPart.input) === JSON.stringify(incomingPart.input)))) matchingServerIndices.push(i); |
There was a problem hiding this comment.
P2: When a recovered tool input has the same keys in a different insertion order, this comparison fails even though the inputs are equivalent. The assistant then keeps a new ID and can be persisted as a duplicate row; compare inputs structurally or with canonicalized keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At patches/agents@0.19.0.patch, line 60:
<comment>When a recovered tool input has the same keys in a different insertion order, this comparison fails even though the inputs are equivalent. The assistant then keeps a new ID and can be persisted as a duplicate row; compare inputs structurally or with canonicalized keys.</comment>
<file context>
@@ -45,36 +45,37 @@ index 8d5b57034f927298d207d8a2bcbac927614780f5..9cf49a4471b9da862ddde11b818fd1ee
-+ id: serverMessage.id
-+ };
-+ }
++ if (serverMessage.role === "assistant" && incomingToolCalls.some((incomingPart) => serverMessage.parts.some((serverPart) => "toolCallId" in serverPart && serverPart.toolCallId === incomingPart.toolCallId && JSON.stringify(serverPart.input) === JSON.stringify(incomingPart.input)))) matchingServerIndices.push(i);
+ }
-+ return incomingMessage;
</file context>
| } | ||
| cards[cardIndex] = { | ||
| ...card, | ||
| [edit.side]: parseFlashcardSideHtml(replaced.text), |
There was a problem hiding this comment.
P2: When replace_text grows a side beyond 8,000 characters, this path stores it because it validates only each edit's replacement, not the resulting HTML. Validate replaced.text with flashcardSideHtmlSchema before parsing, matching update and replace limits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/flashcards/flashcard-edits.ts, line 126:
<comment>When `replace_text` grows a side beyond 8,000 characters, this path stores it because it validates only each edit's replacement, not the resulting HTML. Validate `replaced.text` with `flashcardSideHtmlSchema` before parsing, matching `update` and `replace` limits.</comment>
<file context>
@@ -1,96 +1,156 @@
+ }
+ cards[cardIndex] = {
+ ...card,
+ [edit.side]: parseFlashcardSideHtml(replaced.text),
+ };
+ } else if (edit.op === "delete") {
</file context>
| [edit.side]: parseFlashcardSideHtml(replaced.text), | |
| \t\t\t\t\t[edit.side]: parseFlashcardSideHtml(flashcardSideHtmlSchema.parse(replaced.text)), |
| findCardIndex(cards, destinationRef, targets, initialRevisions); | ||
| const [card] = cards.splice(cardIndex, 1); | ||
| const destinationIndex = findCardIndex(cards, destinationRef, targets, initialRevisions); | ||
| if (!card) throw new FlashcardEditError("ref_not_found"); |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
card is always defined here because cardIndex was already validated by findCardIndex (which throws ref_not_found or ref_stale on invalid refs), so splice(cardIndex, 1) must return the element. Remove this unreachable defensive branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/flashcards/flashcard-edits.ts, line 144:
<comment>`card` is always defined here because `cardIndex` was already validated by `findCardIndex` (which throws `ref_not_found` or `ref_stale` on invalid refs), so `splice(cardIndex, 1)` must return the element. Remove this unreachable defensive branch.</comment>
<file context>
@@ -1,96 +1,156 @@
- if (!card || insertionIndex === null) throw new CardNotFoundError();
- cards.splice(insertionIndex, 0, card);
+ const destinationIndex = findCardIndex(cards, destinationRef, targets, initialRevisions);
+ if (!card) throw new FlashcardEditError("ref_not_found");
+ cards.splice(edit.beforeRef ? destinationIndex : destinationIndex + 1, 0, card);
+ } else {
</file context>
There was a problem hiding this comment.
0 issues found across 8 files (changes from recent commits).
Requires human review: Auto-approval blocked by 13 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 existing issue remains and 2 new issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">
<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:121">
P2: The reveal effect completes the request on the first run after mount, even when the target block has not rendered yet. Document content loads asynchronously via the Yjs collaboration session (use-document-collaboration-session.ts resolves remotely), so on a fresh navigation to a document block the editor's DOM can be empty when this effect runs. In that case `target` is null, `completeRevealRequest(revealRequest, false)` fires the "This source is no longer available." toast, the request is cleared, and the block is never scrolled to once the content actually arrives — there is no retry. The PDF reveal path (WorkspacePdfViewer) handles this by waiting on an `onLayoutReady` event before completing; the document editor should re-check until the content renders (e.g. on editor `update`/`content-transaction`, or with an `onContentError` guard) before completing with false.</violation>
</file>
<file name="src/features/workspaces/flashcards/flashcard-queries.ts">
<violation number="1" location="src/features/workspaces/flashcards/flashcard-queries.ts:48">
P2: When an item has an older `updatedAt` snapshot in the cache, a failed reset leaves that snapshot with the optimistic empty state. Snapshot all matching query data before the prefix-wide update and restore every key on failure; apply the same rollback pattern to rating mutations.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| `[data-ref="${revealRequest.location.blockId}"]`, | ||
| ); | ||
| target?.scrollIntoView({ block: "center" }); | ||
| completeRevealRequest(revealRequest, Boolean(target)); |
There was a problem hiding this comment.
P2: The reveal effect completes the request on the first run after mount, even when the target block has not rendered yet. Document content loads asynchronously via the Yjs collaboration session (use-document-collaboration-session.ts resolves remotely), so on a fresh navigation to a document block the editor's DOM can be empty when this effect runs. In that case target is null, completeRevealRequest(revealRequest, false) fires the "This source is no longer available." toast, the request is cleared, and the block is never scrolled to once the content actually arrives — there is no retry. The PDF reveal path (WorkspacePdfViewer) handles this by waiting on an onLayoutReady event before completing; the document editor should re-check until the content renders (e.g. on editor update/content-transaction, or with an onContentError guard) before completing with false.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 121:
<comment>The reveal effect completes the request on the first run after mount, even when the target block has not rendered yet. Document content loads asynchronously via the Yjs collaboration session (use-document-collaboration-session.ts resolves remotely), so on a fresh navigation to a document block the editor's DOM can be empty when this effect runs. In that case `target` is null, `completeRevealRequest(revealRequest, false)` fires the "This source is no longer available." toast, the request is cleared, and the block is never scrolled to once the content actually arrives — there is no retry. The PDF reveal path (WorkspacePdfViewer) handles this by waiting on an `onLayoutReady` event before completing; the document editor should re-check until the content renders (e.g. on editor `update`/`content-transaction`, or with an `onContentError` guard) before completing with false.</comment>
<file context>
@@ -107,6 +108,18 @@ function DocumentEditorInstance({
+ `[data-ref="${revealRequest.location.blockId}"]`,
+ );
+ target?.scrollIntoView({ block: "center" });
+ completeRevealRequest(revealRequest, Boolean(target));
+ }, [completeRevealRequest, editor, revealRequest]);
</file context>
| onMutate: async () => { | ||
| await queryClient.cancelQueries({ queryKey: itemQueryKey }); | ||
| const previous = queryClient.getQueryData(viewerQuery.queryKey); | ||
| queryClient.setQueriesData<FlashcardViewerData>({ queryKey: itemQueryKey }, (current) => |
There was a problem hiding this comment.
P2: When an item has an older updatedAt snapshot in the cache, a failed reset leaves that snapshot with the optimistic empty state. Snapshot all matching query data before the prefix-wide update and restore every key on failure; apply the same rollback pattern to rating mutations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/flashcards/flashcard-queries.ts, line 48:
<comment>When an item has an older `updatedAt` snapshot in the cache, a failed reset leaves that snapshot with the optimistic empty state. Snapshot all matching query data before the prefix-wide update and restore every key on failure; apply the same rollback pattern to rating mutations.</comment>
<file context>
@@ -33,27 +35,29 @@ export function useResetFlashcardStudyProgress(input: {
+ await queryClient.cancelQueries({ queryKey: itemQueryKey });
const previous = queryClient.getQueryData(viewerQuery.queryKey);
- queryClient.setQueryData(viewerQuery.queryKey, (current) =>
+ queryClient.setQueriesData<FlashcardViewerData>({ queryKey: itemQueryKey }, (current) =>
current ? { ...current, studyState: createEmptyFlashcardStudyState() } : current,
);
</file context>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| startSession("all", false); | ||
| resetProgress(); | ||
| }, [resetProgress, startSession]); | ||
| const toolbar = useMemo( |
There was a problem hiding this comment.
React Doctor · react-doctor/rerender-memo-before-early-return (warning)
This rebuilds the JSX whenever its dependencies change even on renders that take the early return, so move the JSX into a child component rendered after the early return to skip it
Fix → Move the JSX into a child component rendered after the early return, so renders that take the early return never build it
| ); | ||
| } | ||
|
|
||
| function FlashcardStudySurface({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-giant-component (warning)
Component "FlashcardStudySurface" is over 300 lines long, which is hard to read & change. Split it into a few smaller components.
Fix → Pull each section into its own component so the parent is easier to read, test, and change.
| return []; | ||
| } | ||
| const blockId = parseDocumentAiEditRef(edit.editRef); | ||
| edits.flatMap(getDocumentAiRefs).flatMap((ref) => { |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .flatMap().flatMap() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Requires human review: Auto-approval blocked by 15 unresolved issues from previous reviews.
Re-trigger cubic
1 participant
Summary
Architecture
workspace_item_contents; private ratings use the genericworkspace_item_user_statestable.Review history
This replaces #781 with two focused commits and a clean review surface. The replacement branch's Git tree is byte-for-byte identical to the final tree from #781.
Validation
pnpm verify— formatting, lint, TypeScript, 93 test files / 376 tests, client build, and server build passedpnpm db:check— schema and migration consistency passedUI validation remains with the author, as requested.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.