Skip to content

fix(boolean): unify backend payload to resolve missing answers in PDF…#479

Open
AmazingDude wants to merge 6 commits intoAOSSIE-Org:mainfrom
AmazingDude:fix-boolean-payload
Open

fix(boolean): unify backend payload to resolve missing answers in PDF…#479
AmazingDude wants to merge 6 commits intoAOSSIE-Org:mainfrom
AmazingDude:fix-boolean-payload

Conversation

@AmazingDude
Copy link

@AmazingDude AmazingDude commented Feb 25, 2026

PR Description

When generating Boolean questions on the web app, exporting them as a PDF results in Answer X: undefined. I noticed this happens because the web app never actually fetches the answers (the browser extension handles this via a separate API call to /get_boolean_answer, but Output.jsx didn't).

Instead of adding a messy secondary fetch to the React frontend and dealing with race conditions, I updated the backend to just return the answers alongside the questions in one go.

What changed:

  • Backend (server.py): I updated the /get_boolq route so it passes the generated questions right into the existing AnswerPredictor. Now it returns a single payload with both the questions (output) and the answers (answers). I also cast the boolean values to strings ("True"/"False") so React can render them safely.
  • Frontend (Output.jsx): Tweaked the useEffect to grab the new answers array and map it to the questions by index. I also added a quick fallback ("Answer not found") just in case.

Addressed Issues:

Fixes #471

Screenshots/Recordings:

1. Backend: Postman check showing the new combined payload

Screenshot 2026-02-25 173649

2. Frontend: PDF export finally showing the answers

Screenshot 2026-02-25 173826 Screenshot 2026-02-25 173853

Additional Notes:

Backward Compatibility: I made sure this backend change is strictly additive. By returning a separate "answers" array instead of changing the shape of the existing "output" array, the browser extension and Electron desktop app won't break. They'll just ignore the new data until someone updates their frontends to use it.

Checklist

  • My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • My code follows the project's code style and conventions
  • If applicable, I have made corresponding changes or additions to the documentation
  • If applicable, I have made corresponding changes or additions to tests
  • My changes generate no new warnings or errors
  • I have joined the Discord server and I will share a link to this PR with the project maintainers there
  • I have read the Contribution Guidelines
  • Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.

AI Usage Disclosure

Check one of the checkboxes below:

  • This PR does not contain AI-generated code at all.
  • This PR contains AI-generated code. I have tested the code locally and I am responsible for it.

I have used the following AI models and tools: Used LLMs (Gemini/ChatGPT) to bounce around architectural ideas and double check syntax. Wrote, integrated, and tested the actual implementation manually to ensure cross-platform compatibility.

Summary by CodeRabbit

  • New Features

    • Boolean questions now include answers, full question text, context, and count metadata for richer display.
  • Bug Fixes

    • More robust loading and parsing of stored QA data with graceful fallbacks.
    • Improved handling and validation of boolean, MCQ, and short-question formats to prevent malformed entries.
    • Fixed answer-question pairing and context preservation; UI updates correctly when question type changes.
  • Behavior

    • Stored question type is now treated as a read-only value.

@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Backend now enriches BoolQ responses with predicted boolean answers, source text, and count via a new helper; frontend Output.jsx now robustly parses multiple stored QA shapes (boolq/mcq/short), normalizing questions, answers, and context for rendering and export.

Changes

Cohort / File(s) Summary
Backend BoolQ enrichment
backend/server.py
Added enrich_boolq_output(input_text, max_questions) and refactored /get_boolq and /get_problems to return enriched payloads: { output, answers, text, count } instead of only raw questions.
Frontend Output parsing & normalization
eduaid_web/src/pages/Output.jsx
Replaced stored-question-type state with read-only localStorage value; wrapped localStorage parsing in try/catch; added detection for structured keys (output_boolq, output_mcq, output_shortq); guarded iterations with array checks; normalized BoolQ/MCQ/Short entries to reliably populate question, options, answer (from answers[index] with fallbacks), and context (text/Text).

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Frontend as "Frontend (Output.jsx)"
    participant Backend as "Backend /get_boolq"
    participant Predictor as "BoolQ Generator/Model"

    User->>Frontend: Open Output page / trigger load
    Frontend->>Backend: GET /get_boolq (input_text, max_questions)
    Backend->>Predictor: generate questions and predict boolean answers
    Predictor-->>Backend: questions + boolean answers
    Backend-->>Frontend: JSON { output, answers, text, count }
    Frontend-->>User: Render normalized QA entries (questions + answers) and enable PDF export
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code and found each cue,
Questions now paired with answers true,
Context snug in every line,
PDFs pop out all neat and fine,
Hooray — the pairs belong to you.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title clearly identifies the main change: fixing Boolean question answers in PDFs by unifying backend payload structure.
Linked Issues check ✅ Passed Backend changes return answers alongside questions; frontend parses and maps answers by index with fallback. Issue #471 requirement fully met.
Out of Scope Changes check ✅ Passed All changes directly address issue #471: enrich_boolq_output returns unified payload, Output.jsx parses new structure. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@backend/server.py`:
- Around line 77-87: The /get_problems endpoint must be updated to mirror the
/get_boolq response shape by computing boolean_answers and adding an "answers"
array of "True"/"False" strings; locate the boolean handling in
backend.server.py (the boolean_questions variable and the call to
answer.predict_boolean_answer) and after obtaining raw_answers map them to
string_answers = ["True" if a else "False" for a in raw_answers], then include
"answers": string_answers in the jsonify response while preserving any existing
keys (e.g., keep the old boolean-related fields for backward compatibility).

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc3bf1a and 1f5d997.

📒 Files selected for processing (2)
  • backend/server.py
  • eduaid_web/src/pages/Output.jsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
eduaid_web/src/pages/Output.jsx (1)

175-175: [questionType] dependency has no practical effect — consider whether this is intentional.

questionType is initialized once from localStorage (line 10–12) and setQuestionType is never called anywhere in this component, so questionType never changes after mount. The dependency change from [] to [questionType] therefore does not cause any additional re-runs in practice. If the intent is to re-run the effect when the user navigates between question types without unmounting the component, questionType would need to be kept in sync with a shared state source (e.g., a context or a storage-change event).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` at line 175, The effect's dependency array
was changed to include questionType but questionType is only read once from
localStorage (initialized in the component) and never updated (setQuestionType
unused), so the dependency has no practical effect; either remove questionType
from the useEffect dependency array to reflect that it does not change, or make
questionType a reactive source by syncing it with shared state (e.g., Context,
Redux) or adding a storage-change listener so the effect actually re-runs when
questionType changes — update references to questionType and setQuestionType
accordingly to ensure the chosen approach is consistent across the component
(functions and hooks involving questionType).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eduaid_web/src/pages/Output.jsx`:
- Around line 96-112: The code may push duplicate boolean questions because both
structured keys (output_boolq/output_mcq/output_shortq) and the flat
output+answers can exist in qaPairsFromStorage; update the logic to avoid
double-processing by introducing a flag (e.g., structuredConsumed) that is set
to true when you handle any structured key (handle in the output_boolq branch
where combinedQaPairs is appended), and only run the flat output path when
structuredConsumed is false and questionType === "get_boolq"; alternatively,
explicitly remove/clear stale structured keys
(output_boolq/output_mcq/output_shortq) before writing a new flat output to
storage so combinedQaPairs processing in Output.jsx (qaPairsFromStorage,
output_boolq, output, combinedQaPairs) cannot see both forms; also fix the
truthiness check by validating actual keys exist instead of relying on a default
{} that is always truthy.
- Line 94: Wrap the JSON.parse call that reads localStorage.getItem("qaPairs")
in a try/catch inside the component (where qaPairsFromStorage is computed) to
avoid throwing on malformed JSON; on parse failure set a safe fallback (e.g.,
{}) and optionally clear or log the broken value, then continue to call
setQaPairs as normal (update the code around the qaPairsFromStorage constant in
Output.jsx / the relevant useEffect to perform the try/catch and fallback).
- Around line 140-149: The flat output path for questionType === "get_boolq" (in
the block handling Array.isArray(qaPairsFromStorage.output)) omits the context
field, causing inconsistency with the output_boolq path which uses boolData.Text
|| boolData.text; update the object pushed into combinedQaPairs inside that
forEach (where combinedQaPairs.push({...}) is called) to include context:
qaPairsFromStorage.Text || qaPairsFromStorage.text (or similar null-coalescing
access) so boolean items created in this branch carry the same context field as
output_boolq.

---

Nitpick comments:
In `@eduaid_web/src/pages/Output.jsx`:
- Line 175: The effect's dependency array was changed to include questionType
but questionType is only read once from localStorage (initialized in the
component) and never updated (setQuestionType unused), so the dependency has no
practical effect; either remove questionType from the useEffect dependency array
to reflect that it does not change, or make questionType a reactive source by
syncing it with shared state (e.g., Context, Redux) or adding a storage-change
listener so the effect actually re-runs when questionType changes — update
references to questionType and setQuestionType accordingly to ensure the chosen
approach is consistent across the component (functions and hooks involving
questionType).

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f5d997 and b3572c1.

📒 Files selected for processing (2)
  • backend/server.py
  • eduaid_web/src/pages/Output.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/server.py

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (3)
eduaid_web/src/pages/Output.jsx (3)

104-107: LGTM — hasStructuredKeys gate correctly prevents duplicate boolean entries.

The !hasStructuredKeys guard at line 150 properly funnels the flat output path only when no structured key is present, resolving the prior double-push risk.

Also applies to: 150-151

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 104 - 107, The check using
hasStructuredKeys (computed from qaPairsFromStorage.output_boolq,
qaPairsFromStorage.output_mcq, qaPairsFromStorage.output_shortq) correctly
prevents the flat output path from running when structured keys exist; no code
change needed—keep the !hasStructuredKeys guard around the flat output push (the
logic tied to the output variable) to avoid duplicate boolean entries.

94-100: LGTM — JSON.parse wrapped in try/catch as previously requested.

The fallback to {} on parse failure and the Object.keys(...).length > 0 guard on line 102 together cleanly replace the prior always-truthy || {} guard. Both prior issues are resolved.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 94 - 100, The JSON.parse usage
around localStorage.getItem("qaPairs") (variable qaPairsFromStorage) is
correctly wrapped in a try/catch and falls back to {}, and the later guard using
Object.keys(qaPairsFromStorage).length > 0 is appropriate—no changes required;
keep the try/catch block and the fallback assignment to qaPairsFromStorage as
implemented.

109-123: LGTM — boolean answer mapping and context are now consistent across both paths.

Both the output_boolq structured path (lines 109–123) and the flat output path (lines 150–160) use answers?.[index] ?? "Answer not found" and correctly surface context from Text/text. The fix from the previous review (adding context to the flat path) is present.

Also applies to: 150-160

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 109 - 123, The boolean QA
mapping in the output_boolq branch correctly builds combinedQaPairs using
answers?.[index] ?? "Answer not found" and context from boolData.Text ||
boolData.text; ensure the flat output branch does the exact same: when iterating
the flat output array (the other `questions` path using `output`), push the same
shape into combinedQaPairs with question_type: "Boolean", context sourced from
Text || text, and answer using answers?.[index] ?? "Answer not found" so both
branches are consistent (refer to symbols combinedQaPairs, boolData,
output_boolq, and output).
🧹 Nitpick comments (1)
eduaid_web/src/pages/Output.jsx (1)

150-182: questionType state captured by stale closure — add it to the useEffect dependency array.

questionType (lines 151, 161) is read from component state inside the useEffect at line 93 but is absent from the empty dependency array at line 186. While this is not a functional bug today (state and localStorage are read at the same moment on mount), it violates the react-hooks/exhaustive-deps rule and would silently break if questionType were ever set asynchronously before the effect ran.

The cleanest fix is to read from localStorage directly inside the effect, removing the dependency on state entirely:

♻️ Proposed refactor
  useEffect(() => {
+    const questionType = localStorage.getItem("selectedQuestionType");
     let qaPairsFromStorage = {};
     ...
     if (!hasStructuredKeys && Array.isArray(qaPairsFromStorage.output)) {
       if (questionType === "get_boolq") {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 150 - 182, The effect reading
qaPairsFromStorage currently closes over component state questionType (used in
the useEffect that builds combinedQaPairs), which is not in the dependency
array; to fix this either add questionType to the useEffect dependency array or,
preferably, read questionType from localStorage inside the useEffect so the
effect no longer depends on external state. Locate the useEffect that references
questionType, qaPairsFromStorage and combinedQaPairs and update it to derive
questionType via localStorage.getItem (or parse the stored payload) inside the
effect before using it, or if you choose the alternative, include questionType
in the useEffect dependency array to satisfy react-hooks/exhaustive-deps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@eduaid_web/src/pages/Output.jsx`:
- Around line 104-107: The check using hasStructuredKeys (computed from
qaPairsFromStorage.output_boolq, qaPairsFromStorage.output_mcq,
qaPairsFromStorage.output_shortq) correctly prevents the flat output path from
running when structured keys exist; no code change needed—keep the
!hasStructuredKeys guard around the flat output push (the logic tied to the
output variable) to avoid duplicate boolean entries.
- Around line 94-100: The JSON.parse usage around
localStorage.getItem("qaPairs") (variable qaPairsFromStorage) is correctly
wrapped in a try/catch and falls back to {}, and the later guard using
Object.keys(qaPairsFromStorage).length > 0 is appropriate—no changes required;
keep the try/catch block and the fallback assignment to qaPairsFromStorage as
implemented.
- Around line 109-123: The boolean QA mapping in the output_boolq branch
correctly builds combinedQaPairs using answers?.[index] ?? "Answer not found"
and context from boolData.Text || boolData.text; ensure the flat output branch
does the exact same: when iterating the flat output array (the other `questions`
path using `output`), push the same shape into combinedQaPairs with
question_type: "Boolean", context sourced from Text || text, and answer using
answers?.[index] ?? "Answer not found" so both branches are consistent (refer to
symbols combinedQaPairs, boolData, output_boolq, and output).

---

Nitpick comments:
In `@eduaid_web/src/pages/Output.jsx`:
- Around line 150-182: The effect reading qaPairsFromStorage currently closes
over component state questionType (used in the useEffect that builds
combinedQaPairs), which is not in the dependency array; to fix this either add
questionType to the useEffect dependency array or, preferably, read questionType
from localStorage inside the useEffect so the effect no longer depends on
external state. Locate the useEffect that references questionType,
qaPairsFromStorage and combinedQaPairs and update it to derive questionType via
localStorage.getItem (or parse the stored payload) inside the effect before
using it, or if you choose the alternative, include questionType in the
useEffect dependency array to satisfy react-hooks/exhaustive-deps.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3572c1 and e9f8ea5.

📒 Files selected for processing (1)
  • eduaid_web/src/pages/Output.jsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
eduaid_web/src/pages/Output.jsx (3)

105-108: hasStructuredKeys holds a truthy value, not an explicit boolean.

The || chain evaluates to the first truthy operand (an object or array), so !hasStructuredKeys still works. Casting to boolean makes the intent clearer and avoids confusion if the flag is ever logged or compared strictly.

♻️ Proposed refactor
-      const hasStructuredKeys = 
-        qaPairsFromStorage.output_boolq || 
-        qaPairsFromStorage.output_mcq || 
-        qaPairsFromStorage.output_shortq;
+      const hasStructuredKeys = Boolean(
+        qaPairsFromStorage.output_boolq ||
+        qaPairsFromStorage.output_mcq ||
+        qaPairsFromStorage.output_shortq
+      );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 105 - 108, hasStructuredKeys is
assigned a truthy value (the first truthy operand) instead of an explicit
boolean; change the assignment in Output.jsx so hasStructuredKeys is explicitly
boolean by coercing the OR-chain result (e.g., use
Boolean(qaPairsFromStorage.output_boolq || qaPairsFromStorage.output_mcq ||
qaPairsFromStorage.output_shortq) or !!(…)) so downstream uses, logs, or strict
comparisons of hasStructuredKeys behave predictably; update the declaration that
references hasStructuredKeys to use that boolean coercion.

162-171: MCQ flat path lacks a question-field fallback, unlike the else (short) path.

The else branch uses qaPair.question || qaPair.question_statement || qaPair.Question and a similar chain for answer. The MCQ branch hardcodes only qaPair.question_statement. If any MCQ entry ever uses a different field name (e.g., question), the rendered question will be undefined.

♻️ Proposed defensive fix
-          qaPairsFromStorage.output.forEach((qaPair) => {
-            combinedQaPairs.push({
-              question: qaPair.question_statement,
-              question_type: "MCQ",
-              options: qaPair.options,
-              answer: qaPair.answer,
-              context: qaPair.context,
-            });
-          });
+          qaPairsFromStorage.output.forEach((qaPair) => {
+            combinedQaPairs.push({
+              question: qaPair.question_statement || qaPair.question,
+              question_type: "MCQ",
+              options: qaPair.options,
+              answer: qaPair.answer,
+              context: qaPair.context,
+            });
+          });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 162 - 171, The MCQ branch
handling when questionType === "get_mcq" only uses qaPair.question_statement and
lacks the defensive fallbacks used in the other branch; update the loop that
pushes into combinedQaPairs (the block referencing qaPairsFromStorage,
combinedQaPairs and qaPair) to use the same fallback chains for question and
answer (e.g., qaPair.question || qaPair.question_statement || qaPair.Question
and qaPair.answer || qaPair.Answer || qaPair.correctAnswer) and preserve
options/context as before so missing or differently-cased fields won’t produce
undefined.

10-12: Remove unused setQuestionType setter — convert to plain const.

questionType is initialized from localStorage once and never mutated via setQuestionType. The state variable is only read at line 193 in generateGoogleForm. Simplifying to a plain const eliminates the unused setter and clarifies the value's immutability.

Suggested refactoring
-  const [questionType, setQuestionType] = useState(
-    localStorage.getItem("selectedQuestionType")
-  );
+  const questionType = localStorage.getItem("selectedQuestionType");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 10 - 12, Replace the
unnecessary useState call with a plain constant: remove useState(...) that
defines questionType and setQuestionType, and instead read the saved value once
into a const questionType (using localStorage.getItem("selectedQuestionType"))
so the unused setter setQuestionType is eliminated; update any references (e.g.,
in generateGoogleForm) to use the new const and ensure no other code relies on
stateful updates from setQuestionType or useState.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@eduaid_web/src/pages/Output.jsx`:
- Around line 105-108: hasStructuredKeys is assigned a truthy value (the first
truthy operand) instead of an explicit boolean; change the assignment in
Output.jsx so hasStructuredKeys is explicitly boolean by coercing the OR-chain
result (e.g., use Boolean(qaPairsFromStorage.output_boolq ||
qaPairsFromStorage.output_mcq || qaPairsFromStorage.output_shortq) or !!(…)) so
downstream uses, logs, or strict comparisons of hasStructuredKeys behave
predictably; update the declaration that references hasStructuredKeys to use
that boolean coercion.
- Around line 162-171: The MCQ branch handling when questionType === "get_mcq"
only uses qaPair.question_statement and lacks the defensive fallbacks used in
the other branch; update the loop that pushes into combinedQaPairs (the block
referencing qaPairsFromStorage, combinedQaPairs and qaPair) to use the same
fallback chains for question and answer (e.g., qaPair.question ||
qaPair.question_statement || qaPair.Question and qaPair.answer || qaPair.Answer
|| qaPair.correctAnswer) and preserve options/context as before so missing or
differently-cased fields won’t produce undefined.
- Around line 10-12: Replace the unnecessary useState call with a plain
constant: remove useState(...) that defines questionType and setQuestionType,
and instead read the saved value once into a const questionType (using
localStorage.getItem("selectedQuestionType")) so the unused setter
setQuestionType is eliminated; update any references (e.g., in
generateGoogleForm) to use the new const and ensure no other code relies on
stateful updates from setQuestionType or useState.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9f8ea5 and e4b8c8b.

📒 Files selected for processing (1)
  • eduaid_web/src/pages/Output.jsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
eduaid_web/src/pages/Output.jsx (3)

125-135: MCQ structured path lacks the field-name fallback used elsewhere.

The MCQ structured path (line 128) reads only qaPair.question_statement for the question text, while the MCQ flat path (line 163) and the Short paths (lines 140, 173) use a triple fallback (qaPair.question || qaPair.question_statement || qaPair.Question). If the MCQ backend response ever uses question or Question instead of question_statement, this path would produce undefined questions.

Consider aligning the field-name resolution for consistency:

Suggested fix
         qaPairsFromStorage.output_mcq.questions.forEach((qaPair) => {
           combinedQaPairs.push({
-            question: qaPair.question_statement,
+            question: qaPair.question_statement || qaPair.question || qaPair.Question,
             question_type: "MCQ",
             options: qaPair.options,
-            answer: qaPair.answer,
+            answer: qaPair.answer || qaPair.Answer,
             context: qaPair.context,
           });
         });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 125 - 135, The MCQ branch that
builds combinedQaPairs uses qaPair.question_statement only which can yield
undefined; update the question resolution in the loop over
qaPairsFromStorage.output_mcq.questions (where combinedQaPairs is pushed) to use
the same fallback used elsewhere (e.g., qaPair.question ||
qaPair.question_statement || qaPair.Question) so MCQ items resolve consistently.

91-185: Consider extracting the multi-shape normalization logic into a standalone utility.

The useEffect body (lines 91–185) handles ~6 distinct data shapes with multiple field-name fallbacks. This makes the effect hard to read and test in isolation. Extracting the normalization into a pure function (e.g., normalizeQaPairs(raw, questionType)) would allow unit testing the parsing logic independently of React lifecycle, and keep the component focused on rendering.

This isn't urgent but would pay off as the number of supported shapes grows.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 91 - 185, The effect in
useEffect is doing complex multi-shape normalization of qaPairsFromStorage;
extract that logic into a pure helper named normalizeQaPairs(raw, questionType)
that accepts the raw localStorage object and returns the unified combinedQaPairs
array, then replace the big parsing block with: const combinedQaPairs =
normalizeQaPairs(qaPairsFromStorage, questionType); and call
setQaPairs(combinedQaPairs). Ensure normalizeQaPairs handles the same branches
(output_boolq, output_mcq, output_shortq, legacy output with answers/text,
fallbacks for field names like question/question_statement/Question,
answer/Answer/correctAnswer, and options/context) and add unit tests for
normalizeQaPairs.

140-142: Nit: || coerces falsy answers (empty string, 0) to the next fallback.

Lines 142, 166, 175 use || for answer fallback chains. If a legitimate answer is "" or 0, it would be silently replaced. Consider using ?? (nullish coalescing) for the answer fields specifically, similar to line 119's answers?.[index] ?? "Answer not found".

This is unlikely to be a practical issue for quiz answers, but it's an inconsistency with the ?? pattern already used elsewhere in this same effect.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eduaid_web/src/pages/Output.jsx` around lines 140 - 142, Replace the loose OR
fallbacks with nullish coalescing for answer fields (and question if you want
consistent behavior) so empty-string or 0 answers aren’t overwritten: update
occurrences like qaPair.answer || qaPair.Answer (and the similar chains on lines
142, 166, 175) to use qaPair.answer ?? qaPair.Answer (and for question use
qaPair.question ?? qaPair.question_statement ?? qaPair.Question) so only
null/undefined fall through to the next fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@eduaid_web/src/pages/Output.jsx`:
- Around line 125-135: The MCQ branch that builds combinedQaPairs uses
qaPair.question_statement only which can yield undefined; update the question
resolution in the loop over qaPairsFromStorage.output_mcq.questions (where
combinedQaPairs is pushed) to use the same fallback used elsewhere (e.g.,
qaPair.question || qaPair.question_statement || qaPair.Question) so MCQ items
resolve consistently.
- Around line 91-185: The effect in useEffect is doing complex multi-shape
normalization of qaPairsFromStorage; extract that logic into a pure helper named
normalizeQaPairs(raw, questionType) that accepts the raw localStorage object and
returns the unified combinedQaPairs array, then replace the big parsing block
with: const combinedQaPairs = normalizeQaPairs(qaPairsFromStorage,
questionType); and call setQaPairs(combinedQaPairs). Ensure normalizeQaPairs
handles the same branches (output_boolq, output_mcq, output_shortq, legacy
output with answers/text, fallbacks for field names like
question/question_statement/Question, answer/Answer/correctAnswer, and
options/context) and add unit tests for normalizeQaPairs.
- Around line 140-142: Replace the loose OR fallbacks with nullish coalescing
for answer fields (and question if you want consistent behavior) so empty-string
or 0 answers aren’t overwritten: update occurrences like qaPair.answer ||
qaPair.Answer (and the similar chains on lines 142, 166, 175) to use
qaPair.answer ?? qaPair.Answer (and for question use qaPair.question ??
qaPair.question_statement ?? qaPair.Question) so only null/undefined fall
through to the next fallback.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4b8c8b and 5d53ddd.

📒 Files selected for processing (1)
  • eduaid_web/src/pages/Output.jsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Missing answers for Boolean questions in Output.jsx causes "undefined" in PDF export

1 participant