fix(mobile): reject partially numeric line params in the comment composer

[iscekic]

Changelog for users

• A deep link whose line or startLine is partially numeric now opens no comment composer instead of posting to a truncated line.

Changelog for maintainers

• The composer's local parsePositiveInt is gone; number, line, and startLine all use the shared parsePositiveIntParam, so a segment like 1.5 or 12abc is rejected, not truncated.
• commentTrailingControlsClearanceDp is removed; the clearance arithmetic now lives only in the test, which pins the row's gap-3 and both slop objects and asserts 5.5dp.
• The tap-area test reads comment-row.tsx and the pill source from disk and strips comments first, so a comment naming a class no longer satisfies the guard.
• The test imports node:fs and node:url under vitest; reviewer should confirm it never bundles into the app.
• The row's gap stays a literal class because NativeWind resolves it at build time, so the test, not the helper, is the guard against a gap change.
• Review the new removal of the exported helper for any remaining importers before merge.

E2E proof

The comment-composer's local parsePositiveInt uses Number.parseInt, so a deep link with a partial-numeric line or startLine (line=1.5, line=12abc, startLine=9x on the GitHub route) is accepted as 1/12/9 and an inline comment is opened and posted to a line the user never selected; route-params.ts already ships parsePositiveIntParam for exactly this guard, and only the layout-validated number segment is safe.

Code trace: apps/mobile/src/lib/pr-review/comment-composer-params.ts:1 changed in 814266ede3ce7b8e294ff6ce774fb2759b2167dc. Sense check (jev): probability 0.93

Changed lines

-import { parseParam } from '@/lib/route-params';
+import { parseParam, parsePositiveIntParam } from '@/lib/route-params';
-function parsePositiveInt(value: string | string[] | undefined): number | null {
-  const text = parseParam(value);
-  if (!text) {
-    return null;
-  }
-  const parsed = Number.parseInt(text, 10);
-  if (!Number.isInteger(parsed) || parsed <= 0) {
-    return null;
-  }
-  return parsed;
-}
-
- * non-positive line, or startLine greater than line).
+ * non-positive or partially numeric line or startLine, or startLine greater
+ * than line). The numeric params go through `parsePositiveIntParam` so a
+ * segment like `1.5` or `12abc` is rejected rather than silently truncated to
+ * a line the user never selected.
-  const number = parsePositiveInt(raw.number);
+  const number = parsePositiveIntParam(raw.number);
-  const line = parsePositiveInt(raw.line);
-  const startLine = parsePositiveInt(raw.startLine);
+  const line = parsePositiveIntParam(raw.line);
+  const startLine = parsePositiveIntParam(raw.startLine);

commentTrailingControlsClearanceDp is dead production code: no component calls it (comment-row.tsx only names it in a comment) and only its own unit test does, so the test recomputes the expression instead of guarding the row's actual gap-3 and hit slops and cannot catch a change to them.

Code trace: apps/mobile/src/lib/pr-review/comment-trailing-controls.ts:12 changed in 814266ede3ce7b8e294ff6ce774fb2759b2167dc. Sense check (model): The diff deletes the unreferenced export function commentTrailingControlsClearanceDp() in comment-trailing-controls.ts:12 and rewrites the stale comment, removing exactly the dead production code the claim names.

Changed lines

-// session, never the moderation sheet. Keeping the arithmetic in one place lets
-// the row's gap and the two tap areas be checked together (vr1, 2026-09-16).
+// session, never the moderation sheet. The row keeps the literal classes and
+// the slop objects below; `comment-trailing-controls.test.ts` reads the row's
+// source and these two slop objects together so the gap and the two tap areas
+// are checked against what the row actually renders (vr1, 2026-09-16).
-
-/**
- * Horizontal dp between the pill's expanded right edge and the overflow's
- * expanded left edge: the gap between the two frames less each control's slop
- * on the facing side. The overflow's frame is wider than its visible circle,
- * but the gap and both slops are measured from the frame, so the frame's inset
- * does not enter here. A positive value means the two tap areas never overlap,
- * so each control keeps every tap that lands on it.
- */
-export function commentTrailingControlsClearanceDp(): number {
-  return (
-    COMMENT_TRAILING_CONTROLS_GAP_DP - FIX_WITH_KILO_HIT_SLOP.right - COMMENT_ACTIONS_HIT_SLOP.left
-  );
-}

Owner request

Fix 2 janitor findings in mobile/pr-review. Fix every one; the proof covers each.

1. The comment-composer's local parsePositiveInt uses Number.parseInt, so a deep link with a partial-numeric line or startLine (line=1.5, line=12abc, startLine=9x on the GitHub route) is accepted as 1/12/9 and an inline comment is opened and posted to a line the user never selected; route-params.ts already ships parsePositiveIntParam for exactly this guard, and only the layout-validated number segment is safe.
   Trace: apps/mobile/src/lib/pr-review/comment-composer-params.ts:32: The comment-composer's local parsePositiveInt uses Number.parseInt, so a deep link with a partial-numeric line or startLine (line=1.5, line=12abc, startLine=9x on the GitHub route) is accepted as 1/12/9 and an inline comment is opened and posted to a line the user never selected; route-params.ts already ships parsePositiveIntParam for exactly this guard, and only the layout-validated number segment is safe. (janitor area maintainability).
   Files: apps/mobile/src/lib/pr-review/comment-composer-params.ts.
2. commentTrailingControlsClearanceDp is dead production code: no component calls it (comment-row.tsx only names it in a comment) and only its own unit test does, so the test recomputes the expression instead of guarding the row's actual gap-3 and hit slops and cannot catch a change to them.
   Trace: apps/mobile/src/lib/pr-review/comment-trailing-controls.ts:60: commentTrailingControlsClearanceDp is dead production code: no component calls it (comment-row.tsx only names it in a comment) and only its own unit test does, so the test recomputes the expression instead of guarding the row's actual gap-3 and hit slops and cannot catch a change to them. (janitor area maintainability).
   Files: apps/mobile/src/lib/pr-review/comment-trailing-controls.ts, apps/mobile/src/lib/pr-review/comment-trailing-controls.test.ts.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The comment-composer now routes number/line/startLine through the shared parsePositiveIntParam, correctly rejecting partially numeric segments (1.5, 12abc, 9x), and the rewritten trailing-controls test now pins the row's literal gap-3 and both hit-slop objects instead of re-deriving a removed helper; the removed export has no remaining importers.

Files Reviewed (5 files)

• apps/mobile/src/components/pr-review/discussion/comment-row.tsx
• apps/mobile/src/lib/pr-review/comment-composer-params.ts
• apps/mobile/src/lib/pr-review/comment-composer-params.test.ts
• apps/mobile/src/lib/pr-review/comment-trailing-controls.ts
• apps/mobile/src/lib/pr-review/comment-trailing-controls.test.ts

---

_{Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0}

_{Review guidance: REVIEW.md from base branch main}