Additional context accepted from qna to backend#404
Open
shmbhvi101 wants to merge 2 commits into
Open
Conversation
Contributor
WalkthroughQ&A “extras” (additional context + attachment metadata) are now tracked as structured objects, persisted per-recipe in localStorage, sent to backend as a separate extras argument via Changes
Sequence DiagramsequenceDiagram
participant User
participant QnAPage
participant LocalStorage
participant SpecService
participant Backend
User->>QnAPage: Open Q&A page
QnAPage->>LocalStorage: getItem("qa_extras_${recipeId}")
LocalStorage-->>QnAPage: additionalContext + attachments (QaAttachmentRef[])
User->>QnAPage: Edit context / add/remove attachments
QnAPage->>LocalStorage: setItem("qa_extras_${recipeId}", extras)
LocalStorage-->>QnAPage: persisted
User->>QnAPage: Submit answers
QnAPage->>SpecService: submitAnswers(recipeId, answers, { additionalContext, attachments })
SpecService->>Backend: POST /answers (includes additional_context & attachments)
Backend-->>SpecService: 200 OK
SpecService-->>QnAPage: submission result
User->>QnAPage: Trigger regenerate spec
QnAPage->>SpecService: updateQaSubmissionExtras(recipeId, extras) (await)
SpecService->>Backend: POST /qa-submission (additional_context, attachments)
Backend-->>SpecService: 200 OK
SpecService->>Backend: POST /regenerate-spec
Backend-->>SpecService: spec regenerated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(main)/task/[taskId]/qna/page.tsx:
- Around line 172-208: When recipeId changes the component must clear or gate
extras to avoid leaking previous task data: in the restore useEffect (the one
that reads `qa_extras_${recipeId}` and calls setState) initialize/reset
additionalContext and attachments (e.g., set additionalContext = "" and
attachments = []) before attempting to read, or add a "restored" flag to
component state that you set true after the restore completes; then update the
mirror useEffect (the one that writes `qa_extras_${recipeId}` via
localStorage.setItem) to early-return unless recipeId is present and the restore
flag is true (or the init/reset has run). Reference: setState usage in the
restore effect, the `qa_extras_${recipeId}` localStorage key, and the mirror
effect that calls localStorage.setItem.
In `@app/`(main)/task/[taskId]/spec/page.tsx:
- Around line 1017-1039: persistQaExtrasFromLocalStorageIfAny currently returns
early when localStorage has no extras or when sync to backend fails, allowing
subsequent regenerateSpec to reuse stale qa_additional_context/qa_attachments;
change persistQaExtrasFromLocalStorageIfAny to explicitly call
SpecService.updateQaSubmissionExtras(id, { additionalContext: null, attachments:
[] }) (or the API's explicit "clear" payload) when parsed data is empty, and on
failure throw or return a rejected promise/false so callers (e.g.,
regenerateSpec) can cancel/retry the regenerate action instead of proceeding
with stale data; update callers of persistQaExtrasFromLocalStorageIfAny to check
the result or catch the thrown error and abort/retry regenerateSpec accordingly.
In `@services/SpecService.ts`:
- Around line 197-204: The request currently omits keys when the user clears
fields (converting empty values to undefined), which prevents the backend from
being told to clear persisted state; update the construction of the
SubmitRecipeAnswersRequest so that when extras is present you send explicit
clear markers: set additional_context to null when extras exists but
additionalContext is empty (e.g. additional_context: extras ?
(extras.additionalContext?.trim() || null) : undefined) and set attachments to
an empty array when extras exists but attachments is empty (e.g. attachments:
extras ? (extras.attachments && extras.attachments.length > 0 ?
extras.attachments : []) : undefined); make the same change for the other
request construction block referenced (the block around the other
SubmitRecipeAnswersRequest use).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b56ed2f8-0212-4c22-8fcf-cc0335c6bb48
📒 Files selected for processing (5)
app/(main)/task/[taskId]/qna/page.tsxapp/(main)/task/[taskId]/spec/page.tsxlib/types/spec.tsservices/SpecService.tstypes/question.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/(main)/task/[taskId]/spec/page.tsx (1)
1327-1331: Minor: Dead code branch.The check
if (!extrasPersisted)is unreachable sincepersistQaExtrasFromLocalStorageIfAnynow either returnstrueor throws an error. However, this is harmless defensive coding.Simplification (optional)
- const extrasPersisted = - await persistQaExtrasFromLocalStorageIfAny(recipeId); - if (!extrasPersisted) { - throw new Error("Failed to sync QA extras before regenerate"); - } + await persistQaExtrasFromLocalStorageIfAny(recipeId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(main)/task/[taskId]/spec/page.tsx around lines 1327 - 1331, The check for extrasPersisted after calling persistQaExtrasFromLocalStorageIfAny is dead code because that function now either returns true or throws; remove the unnecessary if-block that throws on falsy extrasPersisted (or alternatively update persistQaExtrasFromLocalStorageIfAny to return false instead of throwing if you intend to keep the check). Locate the call to persistQaExtrasFromLocalStorageIfAny and the variable extrasPersisted in page.tsx and remove the unreachable branch to simplify the flow.services/SpecService.ts (1)
239-247: Redundant conditional:extrasis always defined here.The
extrasparameter is required (not optional) inupdateQaSubmissionExtras, so theextras ?ternary check is always true. This makes the code harder to read without providing any benefit.Suggested simplification
{ - additional_context: extras - ? extras.additionalContext?.trim() || null - : undefined, - attachments: - extras - ? extras.attachments && extras.attachments.length > 0 - ? extras.attachments - : [] - : undefined, + additional_context: extras.additionalContext?.trim() || null, + attachments: + extras.attachments && extras.attachments.length > 0 + ? extras.attachments + : [], },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/SpecService.ts` around lines 239 - 247, In updateQaSubmissionExtras, remove the redundant extras ? ternary checks (extras is required) and simplify: set additional_context to extras.additionalContext?.trim() || null and set attachments to extras.attachments && extras.attachments.length > 0 ? extras.attachments : [] (no undefined branch). Update the object properties additional_context and attachments accordingly so they directly use the extras fields instead of wrapping them in an extras ? ... : undefined conditional.
🤖 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/`(main)/task/[taskId]/spec/page.tsx:
- Around line 1327-1331: The check for extrasPersisted after calling
persistQaExtrasFromLocalStorageIfAny is dead code because that function now
either returns true or throws; remove the unnecessary if-block that throws on
falsy extrasPersisted (or alternatively update
persistQaExtrasFromLocalStorageIfAny to return false instead of throwing if you
intend to keep the check). Locate the call to
persistQaExtrasFromLocalStorageIfAny and the variable extrasPersisted in
page.tsx and remove the unreachable branch to simplify the flow.
In `@services/SpecService.ts`:
- Around line 239-247: In updateQaSubmissionExtras, remove the redundant extras
? ternary checks (extras is required) and simplify: set additional_context to
extras.additionalContext?.trim() || null and set attachments to
extras.attachments && extras.attachments.length > 0 ? extras.attachments : []
(no undefined branch). Update the object properties additional_context and
attachments accordingly so they directly use the extras fields instead of
wrapping them in an extras ? ... : undefined conditional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6a69e81-5306-47f9-876f-311cb1aec3d9
📒 Files selected for processing (4)
app/(main)/task/[taskId]/qna/page.tsxapp/(main)/task/[taskId]/spec/page.tsxlib/types/spec.tsservices/SpecService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/types/spec.ts
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
1 participant
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.
Previously, the Additional context attached by user in QnA page, was not being accepted as context for Spec generation by the backend (Image/Pdfs + Text). Now the media is successfully being uploaded and sent to the backend as an additional context for spec gen along with QnA Answers submission.
Summary by CodeRabbit
New Features
Refactor