[lab 5 test] Eight files, reviewer with the per-file fan-out

[sergical]

Test pull request for lab 5. Do not merge. The reviewer in this branch has the per-file fan-out regression.

Created with Claude Code

[github-actions]

Verdict

Request changes — the fixture files contain several real, demonstrable bugs (off-by-one, unit conversion, rounding order), and the reviewer-orchestration change in review.ts has a scalability/heuristic-fragility concern worth addressing before merge.

Correctness Findings (by severity)

1. cart-total.ts — out-of-bounds array access (high).
   for (let i = 0; i <= items.length; i++) iterates one past the end of the array. On the last iteration items[items.length] is undefined, and .price/.qty access throws a TypeError. Should be i < items.length.

2. refund-window.ts — wrong time unit divisor (high).
   Date#getTime() returns milliseconds, but the code divides by 86400 (seconds-per-day) instead of 86400000 (ms-per-day). This makes days ~1000x too small, so the "30 day" refund window actually allows refunds for ~82 years after delivery — the guard is effectively broken.

3. loyalty.ts — rounding applied to the wrong operand (medium-high).
   Math.round(totalCents / 100) * 1.5 rounds the dollar amount before applying the points multiplier, and the final result isn't rounded to an integer either. E.g. totalCents = 149 → Math.round(1.49) * 1.5 = 1.5 instead of the presumably intended ~2.235 (or Math.round(1.49 * 1.5)). This produces systematically wrong point totals and a non-integer point count.

4. discount.ts — ambiguous unit for percent (medium).
   total - total * percent assumes percent is a fraction (0–1). If any caller passes a "20" meaning 20%, the result goes deeply negative. No validation/normalization exists, so misuse fails silently rather than erroring.

5. order-id.ts — parseInt missing radix and no error handling (medium).
   parseInt(last) without a radix can misparse strings like "010", and non-numeric/prefixed input ("ORD-123") can produce NaN, silently yielding "NaN" as the "next" order id instead of failing loudly.

6. shipping.ts — no input validation for weightKg (low-medium).
   Negative, zero, or NaN weights are not rejected; NaN > 5 is false, so an invalid weight silently returns the light-package base cost (6) rather than surfacing an error.

7. review.ts — orchestration change (medium, design concern).

   • The new one-task-per-file fan-out is unbounded: a large PR could spawn dozens of correctness-reviewer tasks with no cap or fallback, unlike the previous fixed two-task design.
   • Counting literal occurrences of the string "diff --git" to determine file count is fragile — if any file's content itself contains that substring (e.g., a fixture embedding an example diff, as this very PR does with demo-pr/*), the count becomes inaccurate.
   • No explicit rule to guarantee each file is assigned to exactly one task with no gaps/duplicates when the lead partitions work.

Style Findings

• cart-total.ts: inline { price: number; qty: number } type should be extracted to a named CartItem interface for clarity/reuse.
• discount.ts: parameter name percent implies 0–100 but is treated as a 0–1 fraction — rename to rate/fraction or document the expected range.
• email.ts: isEmail overpromises — it only checks for "@". Rename (e.g. containsAtSign) or document that it's a loose heuristic, not real validation.
• loyalty.ts: magic number 1.5 should be a named constant (e.g. POINTS_PER_DOLLAR).
• order-id.ts: use explicit radix parseInt(last, 10); consider renaming last to lastOrderId for clarity.
• refund-window.ts: magic number 86400 should be a named constant reflecting the actual unit used (and would have made the unit bug above more visible).
• shipping.ts: magic numbers 5, 12, 6 should be named constants (HEAVY_THRESHOLD_KG, BASE_COST_LIGHT, BASE_COST_HEAVY).
• review.ts: the extended correctness-reviewer description is a dense run-on sentence; and step 2 of the prompt template packs multiple instructions into one dense paragraph — consider splitting for readability.

Summary

The orchestration/process change in review.ts is reasonable in intent (per-file correctness review) but has a couple of untested edge cases around scaling and the header-counting heuristic. More importantly, essentially every new fixture function has at least one genuine bug (out-of-bounds loop, wrong time unit, misordered rounding, ambiguous unit handling, unsafe parsing, missing validation) — these should be fixed before merge, along with the noted style/naming and magic-number cleanups.

[github-actions]

Verdict

Request changes — multiple confirmed correctness bugs (including a hard runtime crash and a critical unit-conversion bug) plus a duplicate-property bug in the review agent itself; not mergeable as-is.

Correctness findings (blocking)

1. cart-total.ts — crash on every non-empty cart. for (let i = 0; i <= items.length; i++) iterates one past the end, so items[items.length] is undefined and items[i].price throws TypeError. Must be i < items.length (or better, use .reduce).

2. refund-window.ts — unit conversion bug, ~1000x off. getTime() is in milliseconds, but the code divides by 86400 (seconds/day) instead of 86_400_000 (ms/day). days ends up ~1000x too large, so canRefund returns false almost immediately after delivery — the 30-day window is effectively broken. Also no lower-bound check: a now earlier than deliveredAt yields negative "days" and still passes.

3. review.ts — duplicated object property. The useSubagent({...}) call for correctness-reviewer now has agent: CorrectnessReviewer, twice. This is very likely a copy-paste artifact; at minimum it's dead/confusing code, and depending on the linter/tsconfig (no-dupe-keys) it could fail the build.

4. loyalty.ts — rounding applied at the wrong step. Math.round(totalCents / 100) * 1.5 rounds the dollar amount before applying the points multiplier, distorting results for non-round-dollar totals (e.g. 150 cents → rounds to 2 → 3 points instead of 2.25). The final result is also a non-integer, which is likely unintended for a points balance. No handling for negative totalCents.

5. discount.ts — ambiguous/unguarded percent unit. Assumes percent is a 0–1 fraction; if a caller passes a human percentage like 20, the result goes deeply negative. No clamping/validation for out-of-range values.

6. order-id.ts — parseInt without radix and loss of zero-padding. parseInt(last) on a zero-padded ID (e.g. "0099") silently drops the padding ("100" instead of a padded value), and non-numeric input silently produces "NaN" as the new order ID with no error handling.

7. email.ts — trivially weak validation. value.includes("@") accepts clearly invalid strings like "@", "foo@", "@bar". Should at minimum require non-empty local/domain parts and reject multiple @ characters.

8. shipping.ts — boundary/edge-case gaps. The weightKg > 5 boundary charges exactly 5kg at the cheaper rate with a sharp jump right after — verify this matches the intended pricing spec. Also no validation for negative/NaN/Infinity weights (NaN > 5 is false, so invalid input silently gets the cheap rate).

9. review.ts prompt change — scalability/ambiguity of new fan-out strategy. Issuing one correctness-reviewer task per changed file (with the full diff pasted into each) has no stated cap for diffs with many files, and no disambiguation guidance for subagents when isolating "their" file among similarly-named/renamed paths in the diff.

Style findings (non-blocking)

• review.ts: the correctness-reviewer description is now a run-on two-sentence blob — reword into one clear sentence (e.g., "Reviews a single file's changes for logic errors, edge cases, and behavioral regressions, using the full diff as context.").
• review.ts: step 2 of the prompt crams counting logic, batching, and a prohibition into one dense paragraph — consider a short sub-list for readability/maintainability.
• Formatting inconsistency: review.ts uses tabs while new fixture files use 2-space indentation — confirm lint/format config covers both.
• cart-total.ts: qty is abbreviated inconsistently next to fully-spelled price — rename to quantity.
• discount.ts: rename percent to fraction/rate and document expected range (0–1 vs 0–100).
• loyalty.ts: extract magic number 1.5 into a named constant (e.g. POINTS_PER_DOLLAR); consider renaming function to indicate cents input.
• order-id.ts: rename last to lastOrderId for clarity.
• refund-window.ts: extract 86400 into a named, correctly-valued constant (MS_PER_DAY).
• shipping.ts: extract magic numbers 5, 12, 6 into named constants describing the pricing tiers.

Summary

This PR (fixture demo files + the review-agent prompt update) has good intentions in the prompt change (per-file correctness review), but the sample files it ships alongside it contain several genuine, high-severity bugs (crash, 1000x unit error, rounding placement, weak validation) that should not be merged as reference/demo code without fixes. The duplicate agent: key in review.ts should also be cleaned up before merge.