Merged
Conversation
Allow users to correct AI-classified PR sizes (XS–XL) via a popover on Dashboard and Ongoing pages. Feedback is stored in a new pull_request_feedbacks tenant table without overwriting the original AI classification. - Add pull_request_feedbacks schema and migration - Add resource route for upsert with org-scoped auth - Show AI classification details (reason, risk areas) in popover - Optimistic UI with useFetcher for instant feedback - Consolidate shared constants (PR_SIZE_LABELS, PR_SIZE_STYLE, complexitySortingFn) into classify.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove per-user feedback_by column (single feedback per PR suffices), simplify LEFT JOIN queries (direct join instead of subquery), structure AI classification info in popover with badges, and deduplicate risk areas. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a resource route that calls Gemini to auto-generate a concise reason for PR size corrections, and integrate it into the SizeBadgePopover as a single-step UI with size selection, reason textarea, and AI Draft button. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…scapeXml - Replace document.getElementById with controlled useState for textarea - Use Promise.all for independent pullRequests + githubRawData queries - Extract shared escapeXml to app/libs/escape-xml.ts (was duplicated 3x) - Deduplicate FormData construction with local buildFormData helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a PR size feedback workflow: SizeBadgePopover UI, server actions for AI-drafted reasons and feedback persistence, DB schema and migrations for pull_request_feedbacks, centralized XML-escape utility, and expanded classification/sorting exports used across tables. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Popover as SizeBadgePopover
participant Server as pr-size-feedback
participant DB as pull_request_feedbacks
User->>Popover: Select size & enter reason
User->>Popover: Click Save
Popover->>Server: POST {number, repositoryId, correctedComplexity, reason}
Server->>Server: Validate auth & input
Server->>DB: Upsert feedback (composite key)
DB-->>Server: OK
Server-->>Popover: { ok: true }
Popover-->>User: Close popover / update display
sequenceDiagram
participant User
participant Popover as SizeBadgePopover
participant Draft as draft-feedback-reason
participant GH as GitHub Raw Data
participant Gemini as Gemini API
User->>Popover: Click "AI Draft"
Popover->>Draft: POST {number, repositoryId, correctedComplexity}
Draft->>GH: Fetch PR, commits, reviews
GH-->>Draft: Raw data
Draft->>Draft: escapeXml & build XML prompt
Draft->>Gemini: Send SYSTEM + XML prompt
Gemini-->>Draft: Generated reason text
Draft-->>Popover: Return drafted reason
Popover-->>User: Display AI-generated reason
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
- Reset reasonText to server value when popover opens (prevents stale state) - Show error message when AI Draft fails (503/500) - Add tests for escapeXml utility (5 tests) - Add tests for complexitySortingFn (3 tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
app/routes/$orgSlug/ongoing/+columns.tsx (1)
5-6: Switch these imports to~/aliases.Both imports target modules under
app/but still use relative paths. Please align them with the repo alias convention.♻️ Proposed tweak
-import { SizeBadgePopover } from '../+components/size-badge-popover' -import { complexitySortingFn } from '../reviews/+functions/classify' +import { SizeBadgePopover } from '~/app/routes/$orgSlug/+components/size-badge-popover' +import { complexitySortingFn } from '~/app/routes/$orgSlug/reviews/+functions/classify'As per coding guidelines, "Use path aliases with
~/prefix for imports from theapp/directory."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/ongoing/+columns.tsx around lines 5 - 6, Update the two relative imports in +columns.tsx to use the project alias for app-rooted modules: replace the import of SizeBadgePopover (currently from '../+components/size-badge-popover') with '~/routes/$orgSlug/ongoing/+components/size-badge-popover' (or the correct alias path under app) and replace complexitySortingFn (currently from '../reviews/+functions/classify') with the corresponding '~/routes/reviews/+functions/classify' alias; ensure the import specifiers remain SizeBadgePopover and complexitySortingFn and that the paths use the '~/...' alias style used across the repo.app/libs/escape-xml.ts (1)
1-7: Escape apostrophes or narrow this helper’s contract.
escapeXmlnow lives in a shared module, so the current name reads like a general XML escaper. It still leaves'untouched, which is fine for text nodes but unsafe for single-quoted attributes. Either add'here or rename the helper to something narrower likeescapeXmlText.♻️ Proposed tweak
export function escapeXml(s: string): string { return s .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') + .replace(/'/g, ''') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/libs/escape-xml.ts` around lines 1 - 7, The helper escapeXml is misleading because it doesn't escape apostrophes; either add an apostrophe replacement or narrow the name: update the function escapeXml to also replace single quotes by adding a .replace(/'/g, ''') so it safely handles single-quoted attributes, or rename the function to escapeXmlText (and update all call sites) to signal it only targets text nodes; ensure the exported identifier and any imports are updated accordingly.app/routes/$orgSlug/+components/size-badge.tsx (1)
3-3: Use the repo alias for this app import.This is importing from
app/via a relative path. Please switch it to the~/form so the component stays aligned with the repo convention.♻️ Proposed tweak
-import { PR_SIZE_STYLE, getPRComplexity } from '../reviews/+functions/classify' +import { + PR_SIZE_STYLE, + getPRComplexity, +} from '~/app/routes/$orgSlug/reviews/+functions/classify'As per coding guidelines, "Use path aliases with
~/prefix for imports from theapp/directory."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/+components/size-badge.tsx at line 3, The import in size-badge.tsx currently uses a relative path; update the import of PR_SIZE_STYLE and getPRComplexity to use the repo alias form (~/reviews/+functions/classify) instead of '../reviews/+functions/classify' so the component follows the project's path-alias convention; keep the imported symbols PR_SIZE_STYLE and getPRComplexity unchanged.app/routes/$orgSlug/+components/size-badge-popover.tsx (1)
17-22: Switch this shared import to a~/path.This import reaches back into the
app/tree with a relative path. Please use the repo-standard~/app/...alias instead.As per coding guidelines, "Use path aliases with
~/prefix for imports from theapp/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx around lines 17 - 22, The import in size-badge-popover.tsx currently reaches back into the app tree via a relative path; replace the relative import for PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity, and the PRSizeLabel type with the repo-standard alias (use an import starting with "~/app/..." that points to the same module in reviews/+functions/classify) so the component uses the `~/` path alias instead of a relative path.app/routes/$orgSlug/draft-feedback-reason.ts (1)
93-99: Use the shared form parser here too.Like the save route, this org-scoped action is manually unpacking
FormDataand callingsafeParse, so it bypasses the Conform/Zodix validation path the repo standardizes on for form actions.As per coding guidelines, "Use Conform with Zod for type-safe form validation with
parseWithZodfrom@conform-to/zod/v4andzxfrom@coji/zodix/v4" and "All org-scoped routes must callrequireOrgMemberorrequireOrgAdminBEFOREparseWithZod(request.formData())to prevent unauthenticated users from receiving validation errors".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/draft-feedback-reason.ts around lines 93 - 99, Replace the manual FormData unpack + draftSchema.safeParse with the shared Conform/Zod pattern: call requireOrgMember(request, params.orgSlug) first, then parse the form via parseWithZod(request.formData(), draftSchema) (and/or zx helper if used in this repo) instead of manually pulling pullRequestNumber/repositoryId/correctedComplexity and calling safeParse; update the handler to use the parsed result from parseWithZod and handle validation errors the same way other org-scoped routes do.app/routes/$orgSlug/_index/+columns.tsx (1)
6-7: Switch these imports to~/aliases.Both new imports reach into the
app/tree via relative paths. Please use~/app/...here to match the repo's import convention.As per coding guidelines, "Use path aliases with
~/prefix for imports from theapp/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/_index/+columns.tsx around lines 6 - 7, Update the two relative imports so they use the repo alias prefix (~/) instead of traversing the app tree; replace the import for SizeBadgePopover and the import for complexitySortingFn with their corresponding ~/... aliased module paths (keeping the same exported symbols SizeBadgePopover and complexitySortingFn) so the file imports from the app/ directory via the alias convention and the project build lint rules pass.app/routes/$orgSlug/pr-size-feedback.ts (1)
15-23: Use the repo-standard form parser in this org route.This action already authenticates first, but the manual
request.formData()+safeParsepath still bypasses the shared Conform/Zodix parsing flow the repo expects for org-scoped forms. Switching this toparseWithZod(... )+zxkeeps coercion and validation behavior consistent with the rest of the app.As per coding guidelines, "Use Conform with Zod for type-safe form validation with
parseWithZodfrom@conform-to/zod/v4andzxfrom@coji/zodix/v4" and "All org-scoped routes must callrequireOrgMemberorrequireOrgAdminBEFOREparseWithZod(request.formData())to prevent unauthenticated users from receiving validation errors".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/pr-size-feedback.ts around lines 15 - 23, The action currently calls requireOrgMember(...) then manually reads request.formData() and runs feedbackSchema.safeParse; replace that manual path with the repo-standard Conform+Zod flow by calling parseWithZod on the incoming form data using the zx helper: after requireOrgMember returns, call parseWithZod(await request.formData(), { schema: feedbackSchema, zx }) (importing parseWithZod from '@conform-to/zod/v4' and zx from '@coji/zodix/v4') and use its result instead of safeParse so coercion/validation follow the org-wide behavior; keep requireOrgMember before parseWithZod to avoid unauthenticated users receiving validation errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 181-219: The Save/A I Draft flow allows race conditions: disable
the Save button whenever a draft is in-flight (check draftFetcher.state !==
'idle' or isDrafting) so users cannot submit a stale reasonText while
draftFetcher is running, and remove the immediate
setOpen(false)/setSelectedSize(null) from the onClick handler for the Save
Button; instead, wait for the fetcher response and only close/reset when the
server returns { ok: true } (handle this in a useEffect watching fetcher.data or
fetcher.state and check fetcher.data.ok), keep using buildFormData(selectedSize)
and fd.set('reason', reasonText) as before, and ensure draftFetcher.submit and
fetcher.submit cannot be called concurrently for the same selectedSize.
In `@app/routes/`$orgSlug/draft-feedback-reason.ts:
- Around line 160-169: The Gemini request in the GoogleGenAI call (new
GoogleGenAI and ai.models.generateContent) has no timeout/cancellation; update
the generateContent call that uses model 'gemini-3-flash-preview' with the
prompt and SYSTEM_INSTRUCTION to include either config.httpOptions.timeout
(e.g., 10000 ms) or attach a config.abortSignal from an AbortController so the
request is cancelled on timeout; modify the generateContent invocation to pass
the chosen timeout/abort option into the config object to prevent hanging
requests.
In `@db/migrations/tenant/20260311023705.sql`:
- Line 18: The migration currently issues an unconditional DROP TABLE for
pull_request_feedbacks; change the statement to use a guarded form (DROP TABLE
IF EXISTS `pull_request_feedbacks`) so reruns or partial states don't error,
update the migration SQL where the DROP TABLE `pull_request_feedbacks`; line
appears, and run the migration against a production-like DB snapshot to verify
the guarded drop behaves as expected.
- Around line 15-18: The INSERT is failing when multiple rows per
(pull_request_number, repository_id) exist; before inserting into
new_pull_request_feedbacks collapse duplicates from pull_request_feedbacks by
choosing a single deterministic row per key (e.g., the row with the latest
updated_at or highest id). Modify the INSERT ... SELECT to select only one row
per (pull_request_number, repository_id) — for example by joining
pull_request_feedbacks to a subquery that picks max(updated_at) or max(id) per
key — and then insert those deduplicated columns (pull_request_number,
repository_id, original_complexity, corrected_complexity, reason, created_at,
updated_at) into new_pull_request_feedbacks.
---
Nitpick comments:
In `@app/libs/escape-xml.ts`:
- Around line 1-7: The helper escapeXml is misleading because it doesn't escape
apostrophes; either add an apostrophe replacement or narrow the name: update the
function escapeXml to also replace single quotes by adding a .replace(/'/g,
''') so it safely handles single-quoted attributes, or rename the function
to escapeXmlText (and update all call sites) to signal it only targets text
nodes; ensure the exported identifier and any imports are updated accordingly.
In `@app/routes/`$orgSlug/_index/+columns.tsx:
- Around line 6-7: Update the two relative imports so they use the repo alias
prefix (~/) instead of traversing the app tree; replace the import for
SizeBadgePopover and the import for complexitySortingFn with their corresponding
~/... aliased module paths (keeping the same exported symbols SizeBadgePopover
and complexitySortingFn) so the file imports from the app/ directory via the
alias convention and the project build lint rules pass.
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 17-22: The import in size-badge-popover.tsx currently reaches back
into the app tree via a relative path; replace the relative import for
PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity, and the PRSizeLabel type with
the repo-standard alias (use an import starting with "~/app/..." that points to
the same module in reviews/+functions/classify) so the component uses the `~/`
path alias instead of a relative path.
In `@app/routes/`$orgSlug/+components/size-badge.tsx:
- Line 3: The import in size-badge.tsx currently uses a relative path; update
the import of PR_SIZE_STYLE and getPRComplexity to use the repo alias form
(~/reviews/+functions/classify) instead of '../reviews/+functions/classify' so
the component follows the project's path-alias convention; keep the imported
symbols PR_SIZE_STYLE and getPRComplexity unchanged.
In `@app/routes/`$orgSlug/draft-feedback-reason.ts:
- Around line 93-99: Replace the manual FormData unpack + draftSchema.safeParse
with the shared Conform/Zod pattern: call requireOrgMember(request,
params.orgSlug) first, then parse the form via parseWithZod(request.formData(),
draftSchema) (and/or zx helper if used in this repo) instead of manually pulling
pullRequestNumber/repositoryId/correctedComplexity and calling safeParse; update
the handler to use the parsed result from parseWithZod and handle validation
errors the same way other org-scoped routes do.
In `@app/routes/`$orgSlug/ongoing/+columns.tsx:
- Around line 5-6: Update the two relative imports in +columns.tsx to use the
project alias for app-rooted modules: replace the import of SizeBadgePopover
(currently from '../+components/size-badge-popover') with
'~/routes/$orgSlug/ongoing/+components/size-badge-popover' (or the correct alias
path under app) and replace complexitySortingFn (currently from
'../reviews/+functions/classify') with the corresponding
'~/routes/reviews/+functions/classify' alias; ensure the import specifiers
remain SizeBadgePopover and complexitySortingFn and that the paths use the
'~/...' alias style used across the repo.
In `@app/routes/`$orgSlug/pr-size-feedback.ts:
- Around line 15-23: The action currently calls requireOrgMember(...) then
manually reads request.formData() and runs feedbackSchema.safeParse; replace
that manual path with the repo-standard Conform+Zod flow by calling parseWithZod
on the incoming form data using the zx helper: after requireOrgMember returns,
call parseWithZod(await request.formData(), { schema: feedbackSchema, zx })
(importing parseWithZod from '@conform-to/zod/v4' and zx from '@coji/zodix/v4')
and use its result instead of safeParse so coercion/validation follow the
org-wide behavior; keep requireOrgMember before parseWithZod to avoid
unauthenticated users receiving validation errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49cbc9f0-cf7d-402d-b135-a88a462fe273
⛔ Files ignored due to path filters (1)
db/migrations/tenant/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
app/libs/escape-xml.tsapp/routes/$orgSlug/+components/size-badge-popover.tsxapp/routes/$orgSlug/+components/size-badge.tsxapp/routes/$orgSlug/_index/+columns.tsxapp/routes/$orgSlug/_index/+functions/queries.server.tsapp/routes/$orgSlug/draft-feedback-reason.tsapp/routes/$orgSlug/ongoing/+columns.tsxapp/routes/$orgSlug/ongoing/+functions/queries.server.tsapp/routes/$orgSlug/pr-size-feedback.tsapp/routes/$orgSlug/reviews/+functions/classify.tsapp/services/tenant-type.tsbatch/lib/llm-classify.tsdb/migrations/tenant/20260311020433.sqldb/migrations/tenant/20260311023705.sqldb/tenant.sqllab/classify/judge-common.ts
Comment on lines
+15
to
+18
| -- Copy rows from old table "pull_request_feedbacks" to new temporary table "new_pull_request_feedbacks" | ||
| INSERT INTO `new_pull_request_feedbacks` (`pull_request_number`, `repository_id`, `original_complexity`, `corrected_complexity`, `reason`, `created_at`, `updated_at`) SELECT `pull_request_number`, `repository_id`, `original_complexity`, `corrected_complexity`, `reason`, `created_at`, `updated_at` FROM `pull_request_feedbacks`; | ||
| -- Drop "pull_request_feedbacks" table after copying rows | ||
| DROP TABLE `pull_request_feedbacks`; |
There was a problem hiding this comment.
Collapse duplicate feedback rows before copying into the new primary key.
db/migrations/tenant/20260311020433.sql allowed multiple rows per (pull_request_number, repository_id) via feedback_by. This INSERT ... SELECT copies all of them into a table whose PK is only (pull_request_number, repository_id), so any PR with more than one historical feedback row will hit UNIQUE constraint failed and abort the migration.
🛠️ One way to make the copy deterministic
-INSERT INTO `new_pull_request_feedbacks` (`pull_request_number`, `repository_id`, `original_complexity`, `corrected_complexity`, `reason`, `created_at`, `updated_at`) SELECT `pull_request_number`, `repository_id`, `original_complexity`, `corrected_complexity`, `reason`, `created_at`, `updated_at` FROM `pull_request_feedbacks`;
+INSERT INTO `new_pull_request_feedbacks` (
+ `pull_request_number`,
+ `repository_id`,
+ `original_complexity`,
+ `corrected_complexity`,
+ `reason`,
+ `created_at`,
+ `updated_at`
+)
+SELECT
+ `pull_request_number`,
+ `repository_id`,
+ `original_complexity`,
+ `corrected_complexity`,
+ `reason`,
+ `created_at`,
+ `updated_at`
+FROM (
+ SELECT
+ *,
+ ROW_NUMBER() OVER (
+ PARTITION BY `pull_request_number`, `repository_id`
+ ORDER BY `updated_at` DESC, `created_at` DESC
+ ) AS `rn`
+ FROM `pull_request_feedbacks`
+)
+WHERE `rn` = 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/migrations/tenant/20260311023705.sql` around lines 15 - 18, The INSERT is
failing when multiple rows per (pull_request_number, repository_id) exist;
before inserting into new_pull_request_feedbacks collapse duplicates from
pull_request_feedbacks by choosing a single deterministic row per key (e.g., the
row with the latest updated_at or highest id). Modify the INSERT ... SELECT to
select only one row per (pull_request_number, repository_id) — for example by
joining pull_request_feedbacks to a subquery that picks max(updated_at) or
max(id) per key — and then insert those deduplicated columns
(pull_request_number, repository_id, original_complexity, corrected_complexity,
reason, created_at, updated_at) into new_pull_request_feedbacks.
| -- Copy rows from old table "pull_request_feedbacks" to new temporary table "new_pull_request_feedbacks" | ||
| INSERT INTO `new_pull_request_feedbacks` (`pull_request_number`, `repository_id`, `original_complexity`, `corrected_complexity`, `reason`, `created_at`, `updated_at`) SELECT `pull_request_number`, `repository_id`, `original_complexity`, `corrected_complexity`, `reason`, `created_at`, `updated_at` FROM `pull_request_feedbacks`; | ||
| -- Drop "pull_request_feedbacks" table after copying rows | ||
| DROP TABLE `pull_request_feedbacks`; |
There was a problem hiding this comment.
Guard the destructive step with IF EXISTS.
Line 18 drops the table unconditionally. The repo's Atlas migration rule requires DROP TABLE IF EXISTS here so reruns or partially repaired states do not fail unnecessarily.
As per coding guidelines, "Review all Atlas auto-generated SQL migrations. Ensure DROP TABLE includes IF EXISTS. Test destructive operations on production DB-equivalent state before deployment".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/migrations/tenant/20260311023705.sql` at line 18, The migration currently
issues an unconditional DROP TABLE for pull_request_feedbacks; change the
statement to use a guarded form (DROP TABLE IF EXISTS `pull_request_feedbacks`)
so reruns or partial states don't error, update the migration SQL where the DROP
TABLE `pull_request_feedbacks`; line appears, and run the migration against a
production-like DB snapshot to verify the guarded drop behaves as expected.
- Add size definition table (cognitive load × impact scope) to docs - Document current prompt issues (line count bias, missing risk area step, definition gap between classify and draft prompts) - Improve draft-feedback-reason prompt: inject size definitions, restructure with XML tags per Gemini 3 prompting guide Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/routes/$orgSlug/+components/size-badge-popover.tsx (1)
153-176:⚠️ Potential issue | 🟠 MajorKeep the picker locked until drafting/saving has actually resolved.
The earlier race is still here: while
draftFetcheris running, users can change the selected size and still clickSave, so a late draft can populatereasonTextfor the wrong size or a stale reason can be persisted. The popover also still closes before the save action confirms success, which hides failures.🔧 Suggested direction
- const fetcher = useFetcher() + const fetcher = useFetcher<{ ok?: boolean }>() const draftFetcher = useFetcher<{ reason?: string; error?: string }>() + const isSaving = fetcher.state !== 'idle' const [open, setOpen] = useState(false) const [selectedSize, setSelectedSize] = useState<PRSizeLabel | null>(null) const [reasonText, setReasonText] = useState(reason ?? '') @@ + useEffect(() => { + if (fetcher.data?.ok) { + setOpen(false) + setSelectedSize(null) + } + }, [fetcher.data]) + @@ - disabled={fetcher.state !== 'idle'} + disabled={isSaving || isDrafting} @@ - disabled={fetcher.state !== 'idle' || selectedSize == null} + disabled={isSaving || isDrafting || selectedSize == null} @@ fetcher.submit(fd, { method: 'post', action: `/${orgSlug}/pr-size-feedback`, }) - setOpen(false) - setSelectedSize(null) }}Also applies to: 214-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx around lines 153 - 176, The size picker allows changes while a draft/save is still in flight and the popover closes before save confirms; update the component to disable changing sizes and prevent closing the popover whenever draftFetcher or fetcher is not idle (use the existing draftFetcher and fetcher states), and make the Save action wait for the save response before closing the popover or re-enabling controls; specifically, gate setSelectedSize and the button disabled state on draftFetcher.state !== 'idle' || fetcher.state !== 'idle', and move popover-close logic to run only after the save/draft response indicates success (apply the same changes where the duplicate block exists around the selectedSize rendering).
🧹 Nitpick comments (2)
app/routes/$orgSlug/reviews/+functions/classify.test.ts (1)
2-2: Use the repository path alias forclassify.This relative import bypasses the project’s
~/import convention for modules underapp/.♻️ Suggested change
-import { complexitySortingFn, getPRComplexity } from './classify' +import { + complexitySortingFn, + getPRComplexity, +} from '~/app/routes/$orgSlug/reviews/+functions/classify'As per coding guidelines "Use path aliases with
~/prefix for imports from theapp/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/reviews/+functions/classify.test.ts at line 2, The test file is using a relative import for classify; change the import to use the repository path alias with the ~/ prefix so it follows project conventions—replace the './classify' import in app/routes/$orgSlug/reviews/+functions/classify.test.ts with the aliased path to the same module (referencing the classify module and its exported symbols complexitySortingFn and getPRComplexity) using the ~/routes/$orgSlug/reviews/+functions/classify path.app/routes/$orgSlug/+components/size-badge-popover.tsx (1)
17-22: Switch this import to the~/alias.
classifylives underapp/, so this should follow the project alias convention instead of traversing relatively.♻️ Suggested change
import { PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity, type PRSizeLabel, -} from '../reviews/+functions/classify' +} from '~/app/routes/$orgSlug/reviews/+functions/classify'As per coding guidelines "Use path aliases with
~/prefix for imports from theapp/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx around lines 17 - 22, The import currently pulling PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity and the PRSizeLabel type from a relative path should be changed to use the project alias prefix; update the import to use the '~/reviews/+functions/classify' alias (keep the same named imports: PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity, type PRSizeLabel) so the module is resolved via the app/ alias convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 153-176: The size picker allows changes while a draft/save is
still in flight and the popover closes before save confirms; update the
component to disable changing sizes and prevent closing the popover whenever
draftFetcher or fetcher is not idle (use the existing draftFetcher and fetcher
states), and make the Save action wait for the save response before closing the
popover or re-enabling controls; specifically, gate setSelectedSize and the
button disabled state on draftFetcher.state !== 'idle' || fetcher.state !==
'idle', and move popover-close logic to run only after the save/draft response
indicates success (apply the same changes where the duplicate block exists
around the selectedSize rendering).
---
Nitpick comments:
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 17-22: The import currently pulling PR_SIZE_LABELS, PR_SIZE_STYLE,
getPRComplexity and the PRSizeLabel type from a relative path should be changed
to use the project alias prefix; update the import to use the
'~/reviews/+functions/classify' alias (keep the same named imports:
PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity, type PRSizeLabel) so the module
is resolved via the app/ alias convention.
In `@app/routes/`$orgSlug/reviews/+functions/classify.test.ts:
- Line 2: The test file is using a relative import for classify; change the
import to use the repository path alias with the ~/ prefix so it follows project
conventions—replace the './classify' import in
app/routes/$orgSlug/reviews/+functions/classify.test.ts with the aliased path to
the same module (referencing the classify module and its exported symbols
complexitySortingFn and getPRComplexity) using the
~/routes/$orgSlug/reviews/+functions/classify path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9f94fd6-96e7-49c4-ac92-2666d14a6200
📒 Files selected for processing (3)
app/libs/escape-xml.test.tsapp/routes/$orgSlug/+components/size-badge-popover.tsxapp/routes/$orgSlug/reviews/+functions/classify.test.ts
✅ Files skipped from review due to trivial changes (1)
- app/libs/escape-xml.test.ts
Ground the PR size feedback loop in Polanyi's tacit knowledge and Nonaka's SECI model. Each phase now maps to a SECI process (Externalization → Combination → Internalization), clarifying why AI-drafted reasons lower the cost of knowledge externalization. Also update implementation paths to match actual file locations and mark draft prompt improvements as completed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Disable size buttons and Save during both save and AI draft operations - Close popover only after save response confirms (useEffect on fetcher.data) - Add 15s AbortSignal timeout to Gemini generateContent call Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
app/routes/$orgSlug/+components/size-badge-popover.tsx (1)
88-94:⚠️ Potential issue | 🟠 MajorOnly close after an explicit successful save.
This still closes on any
fetcher.data, so a 4xx/5xx payload will dismiss the popover and drop the user's edits. Gate the reset on the success shape frompr-size-feedbackinstead of any response object.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx around lines 88 - 94, The effect currently closes the popover on any fetcher.data; change the guard in the useEffect that watches fetcher.state and fetcher.data so it only closes and resets (call setOpen(false) and setSelectedSize(null)) when fetcher.state === 'idle' AND the response matches the explicit pr-size-feedback success shape (e.g. check fetcher.data?.type === 'pr-size-feedback' and that the success/status flag on that payload is truthy). Update the useEffect condition to verify the response shape from the pr-size-feedback action before dismissing.
🧹 Nitpick comments (2)
app/routes/$orgSlug/+components/size-badge-popover.tsx (1)
17-22: Use the~/alias for the classify import.This relative import is inside
app/, so it should follow the repo alias convention to avoid brittle route-to-route paths.As per coding guidelines,
Use path aliases with ~/ prefix for imports from the app/ directory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx around lines 17 - 22, Replace the relative import of classify utilities with the repository path alias: change the import that currently pulls PR_SIZE_LABELS, PR_SIZE_STYLE, getPRComplexity, and PRSizeLabel from '../reviews/+functions/classify' to use the '~/reviews/+functions/classify' alias so the component (size-badge-popover.tsx) follows the app/ alias convention and avoids brittle route-relative paths.app/routes/$orgSlug/draft-feedback-reason.ts (1)
114-119: Use the repo-standard form parsing here.This action hand-rolls
FormDataextraction withsafeParse, so its validation path now diverges from the rest of the org-scoped routes. Please switch this toparseWithZod+zxinstead of manual coercion.As per coding guidelines,
Use Conform with Zod for type-safe form validation with parseWithZod from@conform-to/zod/v4and zx from@coji/zodix/v4``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/draft-feedback-reason.ts around lines 114 - 119, Replace the manual FormData extraction and draftSchema.safeParse with the repo-standard Conform+Zod flow: call parseWithZod(request, draftSchema) and use zx for coercion (via the zx parser import) to produce the typed values and validation errors instead of hand-parsing; then use the returned parsed data/value and fieldErrors from parseWithZod in place of formData.get(...) and the safeParse result, keeping draftSchema as the schema reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 84-86: The textarea bound to reasonText and the AI Draft button
must respect the shared busy state: use the existing isSaving/isDrafting/isBusy
flags to disable the reasonText textarea and the AI Draft trigger (where the
draft button is rendered) so users cannot edit or start drafts while a
save/draft is pending; additionally, harden the effect that applies draftFetcher
responses (the useEffect that writes fetched draft into reasonText) to ignore
stale responses by tracking a request identifier or incrementing a local
draftNonce before each draft request and only applying draftFetcher.data when
its id/nonce matches the latest, using draftFetcher and fetcher as the sources
to compare. Ensure both UI disabling and stale-response-guard are implemented
(affecting reasonText, AI Draft button, the draft submission call site, and the
useEffect that assigns fetched drafts).
In `@docs/pr-size-feedback-loop.md`:
- Around line 91-95: The doc uses two different table
names—`pullRequestFeedbacks` and `pull_request_feedbacks`—which is inconsistent;
pick the canonical table name used in implementation (choose
`pull_request_feedbacks` or `pullRequestFeedbacks`) and replace all occurrences
in this section (including the other occurrences noted around lines 124-126) to
match that canonical identifier so readers see the exact same table name
throughout; update every instance of `pullRequestFeedbacks` to the chosen form
(or vice versa) and verify related references like "テナント DB の ..." and any
bullets mention the same table name.
- Around line 61-70: The fenced code block in the docs (the 4-item checklist) is
missing a language tag; update the opening fence to include a plain language
token such as "text" (i.e., change ``` to ```text) so markdownlint stops
flagging it and the block renders/validates correctly.
---
Duplicate comments:
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 88-94: The effect currently closes the popover on any
fetcher.data; change the guard in the useEffect that watches fetcher.state and
fetcher.data so it only closes and resets (call setOpen(false) and
setSelectedSize(null)) when fetcher.state === 'idle' AND the response matches
the explicit pr-size-feedback success shape (e.g. check fetcher.data?.type ===
'pr-size-feedback' and that the success/status flag on that payload is truthy).
Update the useEffect condition to verify the response shape from the
pr-size-feedback action before dismissing.
---
Nitpick comments:
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx:
- Around line 17-22: Replace the relative import of classify utilities with the
repository path alias: change the import that currently pulls PR_SIZE_LABELS,
PR_SIZE_STYLE, getPRComplexity, and PRSizeLabel from
'../reviews/+functions/classify' to use the '~/reviews/+functions/classify'
alias so the component (size-badge-popover.tsx) follows the app/ alias
convention and avoids brittle route-relative paths.
In `@app/routes/`$orgSlug/draft-feedback-reason.ts:
- Around line 114-119: Replace the manual FormData extraction and
draftSchema.safeParse with the repo-standard Conform+Zod flow: call
parseWithZod(request, draftSchema) and use zx for coercion (via the zx parser
import) to produce the typed values and validation errors instead of
hand-parsing; then use the returned parsed data/value and fieldErrors from
parseWithZod in place of formData.get(...) and the safeParse result, keeping
draftSchema as the schema reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 779437b2-14a2-4775-aa42-4cab951991e9
📒 Files selected for processing (3)
app/routes/$orgSlug/+components/size-badge-popover.tsxapp/routes/$orgSlug/draft-feedback-reason.tsdocs/pr-size-feedback-loop.md
Comment on lines
+84
to
+86
| const isSaving = fetcher.state !== 'idle' | ||
| const isDrafting = draftFetcher.state !== 'idle' | ||
| const isBusy = isSaving || isDrafting |
There was a problem hiding this comment.
Not all inputs honor the busy state.
reasonText can still be edited while drafting, and the late draft response then overwrites those edits in the effect above. The AI Draft button also ignores isSaving, so draft/save can still overlap. Disable the textarea and draft button on isBusy, or ignore stale draft responses.
Also applies to: 189-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/`$orgSlug/+components/size-badge-popover.tsx around lines 84 - 86,
The textarea bound to reasonText and the AI Draft button must respect the shared
busy state: use the existing isSaving/isDrafting/isBusy flags to disable the
reasonText textarea and the AI Draft trigger (where the draft button is
rendered) so users cannot edit or start drafts while a save/draft is pending;
additionally, harden the effect that applies draftFetcher responses (the
useEffect that writes fetched draft into reasonText) to ignore stale responses
by tracking a request identifier or incrementing a local draftNonce before each
draft request and only applying draftFetcher.data when its id/nonce matches the
latest, using draftFetcher and fetcher as the sources to compare. Ensure both UI
disabling and stale-response-guard are implemented (affecting reasonText, AI
Draft button, the draft submission call site, and the useEffect that assigns
fetched drafts).
…ied PRs - Change badge colors to traffic-light gradient (gray→green→blue→amber→red) - Unify chart colors with badge colors via shared PR_SIZE_COLORS - Add PRSize type (5 sizes only) separate from PRSizeLabel (includes Unclassified) - Show size definition text when a size button is selected in popover - Show "-" badge for unclassified PRs, allowing manual classification via popover Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Simplify Gemini timeout to httpOptions.timeout (replaces AbortSignal) - Disable textarea during save/draft operations - Add language tags to fenced code blocks in docs - Fix table name inconsistency in docs (pullRequestFeedbacks → pull_request_feedbacks) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/routes/$orgSlug/draft-feedback-reason.ts (1)
10-14: Use the repo-standard form validation path here.This action hand-rolls
FormDataextraction plussafeParse, so it bypasses the Conform/Zodix flow used in the org-scoped routes. Please switch this toparseWithZod(await request.formData(), { schema })withzxcoercions so validation behavior and types stay consistent.As per coding guidelines, "Use Conform with Zod for type-safe form validation with
parseWithZodfrom@conform-to/zod/v4andzxfrom@coji/zodix/v4."Also applies to: 114-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/draft-feedback-reason.ts around lines 10 - 14, Replace the manual FormData extraction and zod safeParse with the repo-standard parseWithZod flow: import and call parseWithZod(await request.formData(), { schema: draftSchema, zx }) and remove the hand-rolled coerce/safeParse logic; ensure draftSchema remains the zod object used and that zx coercions are enabled so types and validation match org-scoped routes (apply same change for the other instance at the second location mentioned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/`$orgSlug/draft-feedback-reason.ts:
- Around line 157-161: The code is using rawData?.pullRequest, rawData?.commits,
and rawData?.reviews as if they were already parsed objects; instead deserialize
the TEXT JSON first (e.g., JSON.parse) before passing to extractFileList,
extractCommitMessages, extractReviewComments and before reading .body into the
body variable so that body, fileList, commitMessages, and reviewComments are
populated correctly; update the logic around rawData?.pullRequest,
rawData?.commits, and rawData?.reviews to safely parse (with try/catch or a
safeParse helper) and then call extractFileList, extractCommitMessages,
extractReviewComments and access .body from the parsed objects.
---
Nitpick comments:
In `@app/routes/`$orgSlug/draft-feedback-reason.ts:
- Around line 10-14: Replace the manual FormData extraction and zod safeParse
with the repo-standard parseWithZod flow: import and call parseWithZod(await
request.formData(), { schema: draftSchema, zx }) and remove the hand-rolled
coerce/safeParse logic; ensure draftSchema remains the zod object used and that
zx coercions are enabled so types and validation match org-scoped routes (apply
same change for the other instance at the second location mentioned).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 149eaa44-f889-4981-a297-993a6dce5eaf
📒 Files selected for processing (5)
app/routes/$orgSlug/+components/size-badge-popover.tsxapp/routes/$orgSlug/draft-feedback-reason.tsapp/routes/$orgSlug/reviews/+components/pr-size-chart.tsxapp/routes/$orgSlug/reviews/+functions/classify.tsdocs/pr-size-feedback-loop.md
🚧 Files skipped from review as they are similar to previous changes (1)
- app/routes/$orgSlug/+components/size-badge-popover.tsx
Comment on lines
+157
to
+161
| const body = | ||
| (rawData?.pullRequest as { body?: string } | undefined)?.body ?? '' | ||
| const fileList = extractFileList(rawData?.pullRequest) | ||
| const commitMessages = extractCommitMessages(rawData?.commits) | ||
| const reviewComments = extractReviewComments(rawData?.reviews) |
There was a problem hiding this comment.
Deserialize the raw GitHub payloads before building the prompt.
github_raw_data.pull_request, commits, and reviews are stored as TEXT JSON in db/tenant.sql:116-130, but this code casts them directly to objects/arrays. In practice that leaves body, file paths, commit messages, and review comments empty here, so Gemini is generating the draft reason without most of the PR context.
🔧 Suggested fix
+function parseJson<T>(value: unknown): T | null {
+ if (typeof value !== 'string') return null
+ try {
+ return JSON.parse(value) as T
+ } catch {
+ return null
+ }
+}
+
const [pr, rawData] = await Promise.all([
tenantDb
.selectFrom('pullRequests')
...
- const body =
- (rawData?.pullRequest as { body?: string } | undefined)?.body ?? ''
- const fileList = extractFileList(rawData?.pullRequest)
- const commitMessages = extractCommitMessages(rawData?.commits)
- const reviewComments = extractReviewComments(rawData?.reviews)
+ const pullRequestJson = parseJson<{
+ body?: string
+ files?: { path: string }[]
+ }>(rawData?.pullRequest)
+ const commitsJson = parseJson<{ message?: string }[]>(rawData?.commits)
+ const reviewsJson = parseJson<{ body?: string }[]>(rawData?.reviews)
+
+ const body = pullRequestJson?.body ?? ''
+ const fileList = extractFileList(pullRequestJson)
+ const commitMessages = extractCommitMessages(commitsJson)
+ const reviewComments = extractReviewComments(reviewsJson)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const body = | |
| (rawData?.pullRequest as { body?: string } | undefined)?.body ?? '' | |
| const fileList = extractFileList(rawData?.pullRequest) | |
| const commitMessages = extractCommitMessages(rawData?.commits) | |
| const reviewComments = extractReviewComments(rawData?.reviews) | |
| function parseJson<T>(value: unknown): T | null { | |
| if (typeof value !== 'string') return null | |
| try { | |
| return JSON.parse(value) as T | |
| } catch { | |
| return null | |
| } | |
| } | |
| const pullRequestJson = parseJson<{ | |
| body?: string | |
| files?: { path: string }[] | |
| }>(rawData?.pullRequest) | |
| const commitsJson = parseJson<{ message?: string }[]>(rawData?.commits) | |
| const reviewsJson = parseJson<{ body?: string }[]>(rawData?.reviews) | |
| const body = pullRequestJson?.body ?? '' | |
| const fileList = extractFileList(pullRequestJson) | |
| const commitMessages = extractCommitMessages(commitsJson) | |
| const reviewComments = extractReviewComments(reviewsJson) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/`$orgSlug/draft-feedback-reason.ts around lines 157 - 161, The
code is using rawData?.pullRequest, rawData?.commits, and rawData?.reviews as if
they were already parsed objects; instead deserialize the TEXT JSON first (e.g.,
JSON.parse) before passing to extractFileList, extractCommitMessages,
extractReviewComments and before reading .body into the body variable so that
body, fileList, commitMessages, and reviewComments are populated correctly;
update the logic around rawData?.pullRequest, rawData?.commits, and
rawData?.reviews to safely parse (with try/catch or a safeParse helper) and then
call extractFileList, extractCommitMessages, extractReviewComments and access
.body from the parsed objects.
- Document that risk_areas is free-text with noisy output (deployment: 249, release: 107, CI/CD: 69 vs DB migration: 45) - Propose closed-list approach (auth, DB schema, payment, security, external API) - Add enum + Decision procedure step to improvement roadmap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
riskAreas can be a pre-parsed array from the DB, not always a string. Accept unknown input and handle array, string, and other types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pull_request_feedbacksテナントテーブルに保存（元の AI 分類は上書きしない）-バッジを表示し、手動分類を可能にPRSize型をPRSizeLabelから分離（5段階 vs Unclassified 含む）escapeXmlを共通ユーティリティに抽出（3箇所の重複解消）Test plan
pnpm validate通過（lint, format, typecheck, build, test）-バッジ表示 → クリックで手動分類可能🤖 Generated with Claude Code