feat: enforce custom exam total question limit dynamically

[syed-tp]

Why

The global budget of 200 questions for custom exams was previously not enforced during the builder phase, allowing users to select an invalid number of questions and resulting in a backend error. A proactive UI limitation is required to ensure a smooth, error-free experience.

What

• Calculated dynamic limits in CustomExamBuilderScreen to disable the "+ Add Questionnaire" button when the global quota is reached.
• Clamped the slider in CustomExamSubjectBottomSheet so users cannot select more questions than the remaining limit allows.
• Added descriptive visual context (total usage vs budget and remaining quota hints) across the UI.
• Extracted and localized all new UI strings across English, Arabic, Malayalam, and Tamil ARB files.

[claude]

Claude finished @syed-tp's task in 2m 31s —— View job

---

PR Review Summary

✅ What's good

• Clean OpenSpec change (custom-exam-total-limit-ux) with proposal, design, tasks, and spec deltas — fully compliant with the SDK's spec-driven workflow.
• Root cause well diagnosed (backend rejects over-budget exams) and the UI now proactively guards it in two places: the "+ Add Questionnaire" button (screen) and the slider's maxVal + _saveBlock clamp (bottom sheet) — this closes the "save without touching slider" bypass described in design.md.
• No SDK boundary, design-token, Material/Cupertino, or GoRouter/Riverpod violations — uses AppButton, AppText, Design.of(context) correctly. AppButton already wraps itself in AppSemantics.button, so the new dynamic button remains accessible without extra wrapping.
• All new strings (customExamLimitReached, customExamTotalQuestions, customExamRemainingQuestions) are properly localized across en/ar/ml/ta ARBs, and the generated l10n .dart files match their ARB sources (no hand-editing).
• The double-guard (button disabled client-side in the screen and _saveBlock clamp in the sheet) means the earlier over-budget bypass I traced through (remainingQuota going negative and the clamp collapsing to a hardcoded (5, 5) range) is not actually reachable in the current flow, since the sheet can't be opened once remainingQuota < 5.

🔴 CRITICAL — Must fix before merge

None found.

🟡 WARNING — Should fix

1. Edge case: misconfigured/low maxQuestionsPerTest. In custom_exam_builder_screen.dart:103 (canAddMore = remainingQuota >= 5), if the backend ever returns maxQuestionsPerTest < 5, the builder becomes unusable from the very first load — the button shows "Limit Reached (0/N)" and the user can never add a single block. Worth a defensive floor/guard or at least confirming with backend that max_questions_per_test is always ≥ 5.
2. No test coverage for the new quota/clamping logic (custom_exam_builder_screen.dart, custom_exam_subject_bottom_sheet.dart). This is exactly the kind of off-by-one-prone arithmetic (remainingQuota < 5 ? 5.0 : ..., clamp(5, remainingQuota < 5 ? 5 : remainingQuota)) that benefits from a widget/unit test locking in the boundary behavior (e.g. remaining == 5, remaining == 4, blocks.isEmpty).

🔵 SUGGESTION — Nice to have

1. custom_exam_builder_screen.dart computes usedQuestions/remainingQuota twice in build() — once for the "Total Questions" row (lines 76) and again inside the button IIFE (lines 98-102). Hoisting this to a single calculation at the top of build() would avoid duplication and reduce risk of the two computations drifting apart later.
2. The button-selection logic is written as an inline IIFE (() { ... }(), lines 94-134). Extracting it into a small private method (e.g. _buildAddButton(design, l10n, configAsync, builderState)) would match the existing style of the file (_buildHeader, _buildBottomBar, _buildEmptyState, etc.) and read more clearly.
3. The minimum block size 5 is hardcoded in three separate places (custom_exam_builder_screen.dart, custom_exam_subject_bottom_sheet.dart ×2) — the design doc already flags this as a known trade-off, but a shared constant (e.g. CustomExam.minBlockSize) would remove the duplication cheaply since this PR is already touching all three sites.

Verdict

APPROVED
· Branch

[syed-tp]

@claude review

[claude]

Claude finished @syed-tp's task in 2m 3s —— View job

---

PR Review Summary

Re-reviewed after commit 76270d7, which addressed most of the earlier suggestions (extracted _buildAddButton, hoisted duplicate usedQuestions/remainingQuota calc, introduced a shared QuestionnaireBlock.minQuestions constant). One new issue was introduced in that same cleanup.

✅ What's good

• The dedup/refactor commit is clean: usedQuestions/remainingQuota are now computed once at the top of build() (custom_exam_builder_screen.dart:40-45) instead of twice, and the button IIFE was extracted into _buildAddButton — both previous SUGGESTIONs resolved.
• SDK boundaries respected: exams only imports package:core/core.dart and package:core/data/data.dart, no cross-domain imports.
• Design system usage is clean — AppButton, AppText, Design.of(context) used throughout, no Material/Cupertino widgets, no static token imports.
• Localization is complete and consistent across en/ar/ml/ta ARBs and their generated .dart counterparts (customExamLimitReached, customExamTotalQuestions, customExamRemainingQuestions), all correctly parameterized as ICU placeholders.
• OpenSpec change (custom-exam-total-limit-ux) is present with proposal/design/tasks/spec deltas, satisfying the spec-driven workflow requirement for this new UX capability.

🔴 CRITICAL — Must fix before merge

1. QuestionnaireBlock.minQuestions was set to 1, but the minimum block size is documented and previously implemented as 5. In packages/exams/lib/models/questionnaire_block.dart:5, the new shared constant is static const int minQuestions = 1;. Before this refactor commit, main hardcoded 5 in three places (slider min: 5, _saveBlock clamp floor, _noOfQuestions = 15 default), and the OpenSpec docs added by this same PR still say 5 everywhere:

   • design.md:9,28-30,34: "minimum required block size (5)", "remaining >= 5"
   • proposal.md:8: "below the minimum required (5)"
   • specs/.../spec.md:7,12: "minimum block size (5)"
   • tasks.md:4,10,12: "remainingQuota < 5"

   With the constant now 1, the slider's minimum, the _saveBlock clamp floor, and the initial _noOfQuestions default (custom_exam_subject_bottom_sheet.dart:29, now 1 instead of the old 15) have all silently changed — the button-disable logic (canAddMore = remainingQuota >= QuestionnaireBlock.minQuestions) now only disables when quota is < 1, not < 5. This directly contradicts the shipped spec docs and reverts pre-existing behavior. If the backend still expects a meaningful minimum per block, this reintroduces the class of bug this PR is meant to fix. This looks like an unintentional typo during the "standardize" refactor — should be static const int minQuestions = 5; to match the spec and prior behavior (or the OpenSpec docs need to be updated to 1 if the change is intentional, with rationale).
   Fix this →

🟡 WARNING — Should fix

1. Still no test coverage for the quota/clamping logic in custom_exam_builder_screen.dart and custom_exam_subject_bottom_sheet.dart. This arithmetic (remainingQuota < minQuestions, slider divisions, _saveBlock clamp) is exactly the kind of boundary logic that benefits from a widget/unit test (e.g. remaining == minQuestions, remaining == minQuestions - 1, blocks.isEmpty), especially now that the min-block-size constant has already drifted once during refactoring.
2. Edge case: misconfigured/low maxQuestionsPerTest. In custom_exam_builder_screen.dart:34-38, if the backend returns maxQuestionsPerTest below QuestionnaireBlock.minQuestions, totalLimit is floored up to minQuestions, but usedQuestions could still legitimately exceed it, making remainingQuota negative and the builder permanently unusable from the first load. Worth a defensive check or backend confirmation that max_questions_per_test is always sane.

🔵 SUGGESTION — Nice to have

• None outstanding — the previous suggestions (duplicate quota calculation, IIFE extraction, shared constant for the hardcoded 5) have all been addressed in 76270d7.

Verdict

CHANGES REQUESTED — the minQuestions value regression (CRITICAL #1) should be fixed or explicitly justified against the OpenSpec docs before merge.
· Branch

[syed-tp]

@claude review

[claude]

Claude finished @syed-tp's task in 1m 45s —— View job

---

PR Review Summary

Re-reviewed after commits a46fca9 and 545c1e6, which resolve the CRITICAL from the previous review.

✅ What's good

• CRITICAL from last review is resolved: a46fca9 updated the OpenSpec docs (design.md, proposal.md, spec.md, tasks.md) to say 1 everywhere instead of 5, so QuestionnaireBlock.minQuestions = 1 (packages/exams/lib/models/questionnaire_block.dart:5) is now self-consistent with the shipped spec — no more code/doc drift.
• 545c1e6 simplifies totalLimit to math.max(maxLimit, QuestionnaireBlock.minQuestions) (custom_exam_builder_screen.dart:36), which as a side effect also resolves the previous WARNING about a misconfigured/low maxQuestionsPerTest making the builder permanently unusable — totalLimit now always floors to at least minQuestions, so canAddMore can never get stuck impossible-to-satisfy from a bad config value.
• SDK boundaries, design system usage, and localization completeness all remain correct (unchanged from prior passes) — no new imports or hardcoded strings introduced in these two commits.

🔴 CRITICAL — Must fix before merge

None.

🟡 WARNING — Should fix

1. Please confirm the minimum block size drop from 5 → 1 is an intentional product decision, not just a doc fix-up. Before this PR, main hardcoded a minimum of 5 questions per subject block (slider min: 5, default _noOfQuestions = 15). This PR's refactor commits standardized that into QuestionnaireBlock.minQuestions, but the value landed as 1 rather than 5, and instead of restoring 5, the fix in a46fca9 rewrote the OpenSpec docs to match 1. Functionally this now allows a single-question block, which is a real behavior change from main that isn't mentioned in the PR's "Why"/"What" description. If a 1-question block is genuinely fine product-wise, this is fine as-is — just flagging so it's a deliberate choice rather than a side effect of chasing the review comment.
2. Still no test coverage for the quota/clamping logic in custom_exam_builder_screen.dart and custom_exam_subject_bottom_sheet.dart. Boundary cases worth locking in with a widget/unit test: remaining == minQuestions, remaining == minQuestions - 1, blocks.isEmpty, and maxQuestionsPerTest below minQuestions.

🔵 SUGGESTION — Nice to have

None outstanding — previous suggestions (duplicate quota calc, IIFE extraction, shared constant, totalLimit clamp readability) have all been addressed.

Verdict

APPROVED — the remaining items are non-blocking (WARNING #1 is a product confirmation, not a code defect; WARNING #2 is test coverage debt).
· Branch