fix(queries): clamp agent and candidate page values#335
Conversation
Greptile SummaryThis PR fixes unbounded page values in the
Confidence Score: 4/5Safe to merge — the clamping logic is correct and the new tests validate all the edge cases the fix targets. The Both Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["parsePage(value?)"] --> B{"value falsy?"}
B -- yes --> C["use '1'"]
B -- no --> D["parseInt(value, 10)"]
C --> D
D --> E{"Number.isFinite?"}
E -- no (NaN) --> F["return 1"]
E -- yes --> G["Math.max(parsed, 1)"]
G --> H["Math.min(result, 100_000)"]
H --> I["return pageNum"]
I --> J["offset = (pageNum - 1) × 20"]
J --> K["query.range(offset, offset + 19)"]
Reviews (1): Last reviewed commit: "test(queries): cover invalid page ranges" | Re-trigger Greptile |
| function parsePage(value?: string) { | ||
| const parsed = parseInt(value || "1", 10); | ||
| return Number.isFinite(parsed) | ||
| ? Math.min(Math.max(parsed, 1), MAX_PAGE) | ||
| : 1; | ||
| } |
There was a problem hiding this comment.
parsePage is now defined identically in both agents.ts and candidates.ts. If the clamping logic ever needs to change (e.g. a different MAX_PAGE, or switching from parseInt to Number), it will need to be updated in both places. Extracting it to a shared utility (e.g. src/lib/queries/utils.ts) would eliminate this drift risk.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Fixes #334.
Verification
agents.test.tsandcandidates.test.ts.npm/package runner available, so I could not execute the Vitest files here.