docs: add implementation plan for organizer file-drop import agent - #517
Open
theianjones wants to merge 11 commits into
Open
docs: add implementation plan for organizer file-drop import agent#517theianjones wants to merge 11 commits into
theianjones wants to merge 11 commits into
Conversation
Replaces a stale, unrelated plan.md (soft-delete registrations) with a step-by-step guide for the drag-and-drop organizer file import agent: data model, private R2 upload, Cloudflare Agent (proposal-only), the confirm/apply + undo path, and the drawer/dock/inline-diff UI from the wireframe. Grounded in the existing AI judge-scheduler patterns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Contributor
WalkthroughAdds a complete AI-assisted organizer file-drop import feature: a new ChangesOrganizer File-Drop Import
Sequence Diagram(s)sequenceDiagram
participant Organizer
participant ImportShell
participant UploadAPI as /api/agent-import/upload
participant OrganizerFileImportAgent as OrganizerFileImportAgent (DO)
participant applyOrganizerImportFn
rect rgba(99, 132, 255, 0.5)
note over Organizer,UploadAPI: File drop & upload
Organizer->>ImportShell: drop / select file
ImportShell->>ImportShell: createImportRunFn → importRunId
ImportShell->>UploadAPI: POST file + importRunId
UploadAPI->>UploadAPI: auth, size/type check, SHA-256, R2 put
UploadAPI-->>ImportShell: { key, checksum }
end
rect rgba(255, 159, 64, 0.5)
note over ImportShell,OrganizerFileImportAgent: AI proposal generation
ImportShell->>OrganizerFileImportAgent: start(importRunId, competitionId, routeKind)
OrganizerFileImportAgent->>OrganizerFileImportAgent: readImportFile (R2), loadExistingVolunteers/Events
OrganizerFileImportAgent->>OrganizerFileImportAgent: generateText → propose_* tool calls (state only)
OrganizerFileImportAgent-->>ImportShell: streaming AgentState (proposals)
end
rect rgba(75, 192, 192, 0.5)
note over Organizer,applyOrganizerImportFn: Review & confirm
Organizer->>ImportShell: confirm selected proposals
ImportShell->>applyOrganizerImportFn: apply proposals
applyOrganizerImportFn->>applyOrganizerImportFn: planVolunteerApply / planEventApply
applyOrganizerImportFn->>applyOrganizerImportFn: executeVolunteerInvite / executeEventCreate
applyOrganizerImportFn-->>ImportShell: ApplyImportResult (applied/skipped/failed)
ImportShell->>OrganizerFileImportAgent: markApplied(proposalIds)
ImportShell-->>Organizer: ReceiptView
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Phase 1 + shared lib for the organizer file-drop import agent: - agent_import_runs table (audit/retention/idempotency/undo anchor) + id gen - AI_FILE_IMPORT entitlement key - OrganizerFileImportAgent Durable Object namespace + binding in alchemy - shared Zod schemas (proposals, agent state, tool/apply payloads) - pure validators (volunteer dedup/classification, event validation) - server-only access control mirroring the judge-scheduler 4-layer gating No behavior wired yet (agent class, routes, UI follow). Type-checks clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
The "propose" half of the loop (no writes yet): - OrganizerFileImportAgent Durable Object: parses the dropped file, loads existing volunteers/events, runs a proposal-only LLM loop through AI Gateway, streams proposals + activity to state, with refine() and intent disambiguation (ask_clarification) - private /api/agent-import/upload route (no public URL; PII server-side) + createImportRunFn / loadFileImportContextFn server fns - CSV/TSV/text parsing via papaparse (XLSX/PDF deferred) - server-only context loaders (existing volunteers/events, R2 file read) - wired the DO into server.ts (authenticated WS route) + Env binding + routeTree registration Type-checks clean. Apply/undo (write path) and UI are next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
- applyOrganizerImportFn: the only write path, runs on explicit organizer confirm. Creates volunteer invites via the existing invite flow, skips duplicates, fails no-email rows, records created invitations on the run for idempotent re-apply + the receipt. Event proposals are deferred (skipped with a note) until the event write path lands. - undoImportFn: deletes invitations the import created (only while still pending) and clears the recorded entities. - 23 unit tests for parse + validate (all passing). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Phase 7 UI, mounted once in the organizer competition layout:
- ImportShell: full-page drag overlay + persistent "Import a file" dock,
entitlement-gated, drop-enabled on Volunteers/Judges (MVP). Native HTML5
drag/drop; uploads privately then opens the review drawer.
- ImportReviewDrawer: useAgent stream of proposals as the preview (no checkbox
grid), inline match badges + warnings, per-row exclude, refine-by-prompt,
one Confirm ("emails will send"), receipt + Undo import.
- usePageIntent: derives routeKind/eventId from the active route.
Type-checks clean, Biome clean, 23 lib tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
- architecture.md#AI Agents: new "Organizer file-drop import" subsection (agent, private upload, parse/validate, proposal-only loop, confirm/undo write path, entitlement gating) - organizer-dashboard.md: layout-level "Organizer File-Drop Import" section All wiki-link targets verified to exist as named exports. (lat check not runnable here — lat.md CLI is not installed in this environment.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
12 issues found across 26 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/components/organizer-import/import-shell.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/organizer-import/import-shell.tsx:72">
P2: Concurrent imports are allowed while an upload is already in progress. This can create duplicate import runs and race the drawer state.</violation>
<violation number="2" location="apps/wodsmith-start/src/components/organizer-import/import-shell.tsx:112">
P2: File drop default behavior is not prevented when import is inactive. Users can accidentally leave the app by dropping a file on unsupported pages.</violation>
</file>
<file name="apps/wodsmith-start/src/db/schemas/agent-imports.ts">
<violation number="1" location="apps/wodsmith-start/src/db/schemas/agent-imports.ts:103">
P2: `appliedEntities` uses `text()` and can overflow for large import receipts. This can fail apply finalization when many entities are written.</violation>
</file>
<file name="apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx:81">
P2: Agent run is never canceled when drawer unmounts. Closing the review UI can leave background model work running unnecessarily.</violation>
</file>
<file name="apps/wodsmith-start/src/components/organizer-import/use-page-intent.ts">
<violation number="1" location="apps/wodsmith-start/src/components/organizer-import/use-page-intent.ts:25">
P2: Substring route matching over-enables import intent on non-target volunteer subpages. Narrow this check to the intended volunteers index route.</violation>
</file>
<file name="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts">
<violation number="1" location="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts:181">
P1: Apply is missing optimistic locking/idempotency on the run row; concurrent requests can clobber `appliedEntities` and final status.</violation>
</file>
<file name="apps/wodsmith-start/src/server.ts">
<violation number="1" location="apps/wodsmith-start/src/server.ts:148">
P3: New organizer route duplicates existing agent-auth routing logic instead of reusing a shared helper. Duplicated security checks are likely to drift and cause inconsistent authorization behavior across agents.</violation>
<violation number="2" location="apps/wodsmith-start/src/server.ts:163">
P2: Route authorizes only `<userId>` suffix; unvalidated `<importRunId>` still lets authenticated users create arbitrary organizer-agent DO identities. This enables unbounded DO creation/probing under one account and breaks the intended run-to-agent binding.</violation>
</file>
<file name="apps/wodsmith-start/src/server/organizer-file-import/access.ts">
<violation number="1" location="apps/wodsmith-start/src/server/organizer-file-import/access.ts:72">
P2: `event_detail` requests are accepted without an eventId, so single-event scoping can be bypassed and treated as broader competition scope.</violation>
<violation number="2" location="apps/wodsmith-start/src/server/organizer-file-import/access.ts:151">
P1: Agent access is not bound to import-run ownership, allowing cross-user run access within the same authorized team/competition.</violation>
</file>
<file name="apps/wodsmith-start/src/routes/api/agent-import/upload.ts">
<violation number="1" location="apps/wodsmith-start/src/routes/api/agent-import/upload.ts:45">
P2: FormData entries are type-cast without runtime checks, so malformed multipart input can crash this handler. Validate `file` is a File and `importRunId` is a non-empty string before using them.</violation>
<violation number="2" location="apps/wodsmith-start/src/routes/api/agent-import/upload.ts:65">
P1: Upload accepts non-created runs and can mutate finalized import runs back to `UPLOADED`. This breaks run lifecycle integrity and can desync apply/undo audit state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| appliedByUserId: session?.user.id ?? scope.createdByUserId, | ||
| appliedAt: new Date(), | ||
| }) | ||
| .where(eq(agentImportRunsTable.id, data.importRunId)) |
Contributor
There was a problem hiding this comment.
P1: Apply is missing optimistic locking/idempotency on the run row; concurrent requests can clobber appliedEntities and final status.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts, line 181:
<comment>Apply is missing optimistic locking/idempotency on the run row; concurrent requests can clobber `appliedEntities` and final status.</comment>
<file context>
@@ -0,0 +1,378 @@
+ appliedByUserId: session?.user.id ?? scope.createdByUserId,
+ appliedAt: new Date(),
+ })
+ .where(eq(agentImportRunsTable.id, data.importRunId))
+
+ logInfo({
</file context>
| * session's user id; this function receives that user id and performs the same | ||
| * team-permission + entitlement checks directly from persistent data. | ||
| */ | ||
| export async function requireFileImportAgentAccess( |
Contributor
There was a problem hiding this comment.
P1: Agent access is not bound to import-run ownership, allowing cross-user run access within the same authorized team/competition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server/organizer-file-import/access.ts, line 151:
<comment>Agent access is not bound to import-run ownership, allowing cross-user run access within the same authorized team/competition.</comment>
<file context>
@@ -0,0 +1,226 @@
+ * session's user id; this function receives that user id and performs the same
+ * team-permission + entitlement checks directly from persistent data.
+ */
+export async function requireFileImportAgentAccess(
+ input: { competitionId: string; routeKind: string; eventId?: string | null },
+ userId: string,
</file context>
|
|
||
| // Authorize against the run's competition (defense in depth), then | ||
| // confirm the uploader owns this run. | ||
| const scope = await loadFileImportScopeByRun(importRunId) |
Contributor
There was a problem hiding this comment.
P1: Upload accepts non-created runs and can mutate finalized import runs back to UPLOADED. This breaks run lifecycle integrity and can desync apply/undo audit state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/api/agent-import/upload.ts, line 65:
<comment>Upload accepts non-created runs and can mutate finalized import runs back to `UPLOADED`. This breaks run lifecycle integrity and can desync apply/undo audit state.</comment>
<file context>
@@ -0,0 +1,134 @@
+
+ // Authorize against the run's competition (defense in depth), then
+ // confirm the uploader owns this run.
+ const scope = await loadFileImportScopeByRun(importRunId)
+ await requireFileImportTeamAccess({
+ teamId: scope.organizingTeamId,
</file context>
| // biome-ignore lint/a11y/noStaticElementInteractions: passive page-level file drop zone (progressive enhancement); the keyboard-accessible path is the "Import a file" dock button | ||
| <div | ||
| onDragEnter={(e) => { | ||
| if (!active || !isFileDrag(e)) return |
Contributor
There was a problem hiding this comment.
P2: File drop default behavior is not prevented when import is inactive. Users can accidentally leave the app by dropping a file on unsupported pages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/organizer-import/import-shell.tsx, line 112:
<comment>File drop default behavior is not prevented when import is inactive. Users can accidentally leave the app by dropping a file on unsupported pages.</comment>
<file context>
@@ -0,0 +1,187 @@
+ // biome-ignore lint/a11y/noStaticElementInteractions: passive page-level file drop zone (progressive enhancement); the keyboard-accessible path is the "Import a file" dock button
+ <div
+ onDragEnter={(e) => {
+ if (!active || !isFileDrag(e)) return
+ e.preventDefault()
+ dragDepth.current += 1
</file context>
| const active = dropEnabled && hasAccess === true && intent !== null | ||
|
|
||
| async function handleFiles(files: FileList | null) { | ||
| if (!active || !intent) return |
Contributor
There was a problem hiding this comment.
P2: Concurrent imports are allowed while an upload is already in progress. This can create duplicate import runs and race the drawer state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/organizer-import/import-shell.tsx, line 72:
<comment>Concurrent imports are allowed while an upload is already in progress. This can create duplicate import runs and race the drawer state.</comment>
<file context>
@@ -0,0 +1,187 @@
+ const active = dropEnabled && hasAccess === true && intent !== null
+
+ async function handleFiles(files: FileList | null) {
+ if (!active || !intent) return
+ const file = files?.[0]
+ if (!file) return
</file context>
| if (routeId.includes("/volunteers/judges")) { | ||
| return { routeKind: "judges" } | ||
| } | ||
| if (routeId.includes("/volunteers")) { |
Contributor
There was a problem hiding this comment.
P2: Substring route matching over-enables import intent on non-target volunteer subpages. Narrow this check to the intended volunteers index route.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/organizer-import/use-page-intent.ts, line 25:
<comment>Substring route matching over-enables import intent on non-target volunteer subpages. Narrow this check to the intended volunteers index route.</comment>
<file context>
@@ -0,0 +1,49 @@
+ if (routeId.includes("/volunteers/judges")) {
+ return { routeKind: "judges" }
+ }
+ if (routeId.includes("/volunteers")) {
+ return { routeKind: "volunteers" }
+ }
</file context>
| } | ||
| const ns = | ||
| env.ORGANIZER_FILE_IMPORT_AGENT as unknown as DurableObjectNamespace<OrganizerFileImportAgent> | ||
| const stub = await getAgentByName(ns, name) |
Contributor
There was a problem hiding this comment.
P2: Route authorizes only <userId> suffix; unvalidated <importRunId> still lets authenticated users create arbitrary organizer-agent DO identities. This enables unbounded DO creation/probing under one account and breaks the intended run-to-agent binding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server.ts, line 163:
<comment>Route authorizes only `<userId>` suffix; unvalidated `<importRunId>` still lets authenticated users create arbitrary organizer-agent DO identities. This enables unbounded DO creation/probing under one account and breaks the intended run-to-agent binding.</comment>
<file context>
@@ -143,6 +145,24 @@ const startEntry = createServerEntry({
+ }
+ const ns =
+ env.ORGANIZER_FILE_IMPORT_AGENT as unknown as DurableObjectNamespace<OrganizerFileImportAgent>
+ const stub = await getAgentByName(ns, name)
+ return stub.fetch(request)
+ }
</file context>
|
|
||
| // When targeting a single event, prove the track workout is part of this | ||
| // competition's programming track (prevents cross-competition event ids). | ||
| if (input.eventId) { |
Contributor
There was a problem hiding this comment.
P2: event_detail requests are accepted without an eventId, so single-event scoping can be bypassed and treated as broader competition scope.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server/organizer-file-import/access.ts, line 72:
<comment>`event_detail` requests are accepted without an eventId, so single-event scoping can be bypassed and treated as broader competition scope.</comment>
<file context>
@@ -0,0 +1,226 @@
+
+ // When targeting a single event, prove the track workout is part of this
+ // competition's programming track (prevents cross-competition event ids).
+ if (input.eventId) {
+ const [eventRow] = await db
+ .select({ trackWorkoutId: trackWorkoutsTable.id })
</file context>
| } | ||
|
|
||
| const form = await request.formData() | ||
| const file = form.get("file") as File | null |
Contributor
There was a problem hiding this comment.
P2: FormData entries are type-cast without runtime checks, so malformed multipart input can crash this handler. Validate file is a File and importRunId is a non-empty string before using them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/api/agent-import/upload.ts, line 45:
<comment>FormData entries are type-cast without runtime checks, so malformed multipart input can crash this handler. Validate `file` is a File and `importRunId` is a non-empty string before using them.</comment>
<file context>
@@ -0,0 +1,134 @@
+ }
+
+ const form = await request.formData()
+ const file = form.get("file") as File | null
+ const importRunId = form.get("importRunId") as string | null
+
</file context>
| const stub = await getAgentByName(ns, name) | ||
| return stub.fetch(request) | ||
| } | ||
| if (namespace === "organizer-file-import-agent") { |
Contributor
There was a problem hiding this comment.
P3: New organizer route duplicates existing agent-auth routing logic instead of reusing a shared helper. Duplicated security checks are likely to drift and cause inconsistent authorization behavior across agents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server.ts, line 148:
<comment>New organizer route duplicates existing agent-auth routing logic instead of reusing a shared helper. Duplicated security checks are likely to drift and cause inconsistent authorization behavior across agents.</comment>
<file context>
@@ -143,6 +145,24 @@ const startEntry = createServerEntry({
const stub = await getAgentByName(ns, name)
return stub.fetch(request)
}
+ if (namespace === "organizer-file-import-agent") {
+ // Instance names are `<importRunId>__<userId>`. Reject anything else so
+ // a caller can't materialize arbitrary DO identities by hitting the
</file context>
Split the apply decision logic out of applyOrganizerImportFn into a pure planVolunteerApply() in validate.ts — it decides invite/skip/fail per row (idempotency, duplicates, no-email, no-team) with no DB access, so the server fn only performs IO for the invites. Adds 6 unit tests (29 total). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Extends the import beyond volunteers to creating events from a dropped packet on the Events list page: - pure planEventApply() (create/skip/fail; updates deferred) + 5 tests - executeEventCreate via createWorkoutAndAddToCompetitionFn; undo deletes the created event with removeWorkoutFromCompetitionFn (cascade-aware) - usePageIntent enables the Events list page (event detail/update deferred) - review drawer renders event proposals + mode-aware confirm/receipt - lat.md updated to reflect events are now applied (34 tests total) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Contributor
There was a problem hiding this comment.
5 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx:130">
P3: Undo success copy is hardcoded to invitations, so event imports show incorrect confirmation text after undo.</violation>
</file>
<file name="apps/wodsmith-start/src/lib/organizer-file-import/validate.ts">
<violation number="1" location="apps/wodsmith-start/src/lib/organizer-file-import/validate.ts:164">
P1: Event apply planner is not idempotent within a single request. Duplicate rowKeys in the same batch can create duplicate events.</violation>
</file>
<file name="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts">
<violation number="1" location="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts:180">
P2: Event create path misses in-request idempotency tracking. Repeated event rowKeys can create duplicate events in one apply call.</violation>
<violation number="2" location="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts:180">
P2: Successful volunteer applies are not added to the idempotency set. Duplicate rowKeys in the same apply request can be re-processed instead of skipped.</violation>
<violation number="3" location="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts:265">
P1: Failed event undos are treated as skipped, then all undo metadata is cleared. This can leave imported events in place with no way to retry undo from the run record.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| options: PlanVolunteerApplyOptions, | ||
| ): VolunteerApplyDecision[] { | ||
| return proposals.map((proposal): VolunteerApplyDecision => { | ||
| if (options.alreadyAppliedRowKeys.has(proposal.rowKey)) { |
Contributor
There was a problem hiding this comment.
P1: Event apply planner is not idempotent within a single request. Duplicate rowKeys in the same batch can create duplicate events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/lib/organizer-file-import/validate.ts, line 164:
<comment>Event apply planner is not idempotent within a single request. Duplicate rowKeys in the same batch can create duplicate events.</comment>
<file context>
@@ -126,6 +126,87 @@ export function isActionableVolunteer(proposal: VolunteerProposal): boolean {
+ options: PlanVolunteerApplyOptions,
+): VolunteerApplyDecision[] {
+ return proposals.map((proposal): VolunteerApplyDecision => {
+ if (options.alreadyAppliedRowKeys.has(proposal.rowKey)) {
+ return {
+ rowKey: proposal.rowKey,
</file context>
| }) | ||
| if (!invite || invite.acceptedAt) { | ||
| // Already accepted (now a membership) or already gone — leave it. | ||
| skippedCount++ |
Contributor
There was a problem hiding this comment.
P1: Failed event undos are treated as skipped, then all undo metadata is cleared. This can leave imported events in place with no way to retry undo from the run record.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts, line 265:
<comment>Failed event undos are treated as skipped, then all undo metadata is cleared. This can leave imported events in place with no way to retry undo from the run record.</comment>
<file context>
@@ -216,23 +255,42 @@ export const undoImportFn = createServerFn({ method: "POST" })
+ })
+ if (!invite || invite.acceptedAt) {
+ // Already accepted (now a membership) or already gone — leave it.
+ skippedCount++
+ continue
+ }
</file context>
| } | ||
| const outcome = await executeVolunteerInvite(decision, scope) | ||
| results.push(outcome.result) | ||
| if (outcome.entity) appliedEntities.push(outcome.entity) |
Contributor
There was a problem hiding this comment.
P2: Event create path misses in-request idempotency tracking. Repeated event rowKeys can create duplicate events in one apply call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts, line 180:
<comment>Event create path misses in-request idempotency tracking. Repeated event rowKeys can create duplicate events in one apply call.</comment>
<file context>
@@ -140,29 +152,56 @@ export const applyOrganizerImportFn = createServerFn({ method: "POST" })
}
+ const outcome = await executeVolunteerInvite(decision, scope)
+ results.push(outcome.result)
+ if (outcome.entity) appliedEntities.push(outcome.entity)
}
</file context>
| } | ||
| const outcome = await executeVolunteerInvite(decision, scope) | ||
| results.push(outcome.result) | ||
| if (outcome.entity) appliedEntities.push(outcome.entity) |
Contributor
There was a problem hiding this comment.
P2: Successful volunteer applies are not added to the idempotency set. Duplicate rowKeys in the same apply request can be re-processed instead of skipped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts, line 180:
<comment>Successful volunteer applies are not added to the idempotency set. Duplicate rowKeys in the same apply request can be re-processed instead of skipped.</comment>
<file context>
@@ -140,29 +152,56 @@ export const applyOrganizerImportFn = createServerFn({ method: "POST" })
}
+ const outcome = await executeVolunteerInvite(decision, scope)
+ results.push(outcome.result)
+ if (outcome.entity) appliedEntities.push(outcome.entity)
}
</file context>
| data: { | ||
| importRunId, | ||
| volunteerProposals: isEventMode ? [] : included, | ||
| eventProposals: isEventMode ? includedEvents : [], |
Contributor
There was a problem hiding this comment.
P3: Undo success copy is hardcoded to invitations, so event imports show incorrect confirmation text after undo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx, line 130:
<comment>Undo success copy is hardcoded to invitations, so event imports show incorrect confirmation text after undo.</comment>
<file context>
@@ -109,19 +120,20 @@ export function ImportReviewDrawer({
- volunteerProposals: included,
- eventProposals: [],
+ volunteerProposals: isEventMode ? [] : included,
+ eventProposals: isEventMode ? includedEvents : [],
},
})
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts-1-10 (1)
1-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the required
@latcode reference.This new server-functions module has no
//@lat: [[...]]reference tying the write path to the organizer import spec. Add one near the module header or exported server functions using the actual section id. As per coding guidelines,**/*.{js,ts,rs,go,c,h,py}files must tie source code to concepts using code refs://@lat: [[section-id]].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts` around lines 1 - 10, The organizer-file-import-fns.ts module is missing the required code reference annotation. Add a `// `@lat`: [[section-id]]` comment near the module header (after the existing JSDoc block) or adjacent to the exported server functions like createImportRunFn and loadFileImportContextFn to tie this write path to the organizer import spec. Replace section-id with the actual section identifier from the organizer import specification as per the coding guidelines for source-to-concept traceability.Source: Coding guidelines
apps/wodsmith-start/src/routes/api/agent-import/upload.ts-1-8 (1)
1-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the required
@latcode reference.This new TS source file has no
//@lat: [[...]]reference tying the upload endpoint to the organizer import concept. Add it near the module header or route definition using the actual section id. As per coding guidelines,**/*.{js,ts,rs,go,c,h,py}files must tie source code to concepts using code refs://@lat: [[section-id]].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/routes/api/agent-import/upload.ts` around lines 1 - 8, The TypeScript source file for the upload endpoint is missing the required `// `@lat`: [[section-id]]` code reference that ties this module to the organizer import concept. Add a `// `@lat`: [[...]]` comment near the module header or route definition (within the first few lines after any imports) using the appropriate section id that corresponds to the organizer import functionality. This reference is required by coding guidelines for all source files to establish traceability to architectural concepts.Source: Coding guidelines
apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts-407-414 (1)
407-414:⚠️ Potential issue | 🟡 MinorAdd
scoreTypeto thecreateWorkoutAndAddToCompetitionFncall.
decision.scoreTypefrom the import file is available but not forwarded tocreateWorkoutAndAddToCompetitionFn. SincescoreTypecontrols the scoring mode (min/max/sum) and is optional in the input schema, omitting it may result in imported events without the intended scoring configuration.Context
const { trackWorkoutId } = await createWorkoutAndAddToCompetitionFn({ data: { competitionId: scope.competitionId, teamId: scope.organizingTeamId, name: decision.name, scheme: decision.scheme, description: decision.description ?? undefined, },Add
scoreType: decision.scoreType ?? undefinedto the data object.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts` around lines 407 - 414, The createWorkoutAndAddToCompetitionFn call is missing the scoreType parameter in its data object. Add scoreType from the decision object to the data parameter passed to createWorkoutAndAddToCompetitionFn, using a nullish coalescing operator to handle cases where scoreType may be undefined, ensuring the imported workout's scoring configuration is properly forwarded.apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx-164-166 (1)
164-166:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUndo success message is incorrect for event imports.
Line 165 always says
invitation(s), but undo can also revert created events. This yields incorrect receipts in event mode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx` around lines 164 - 166, The toast success message in the undo operation always displays "invitation(s)" regardless of what was actually undone, but the operation can revert either invitations or events. Update the toast.success call to dynamically construct the message based on what type of objects were undone. Check the result object for additional properties that indicate whether invitations or events were reverted, and use that information to display either "invitation(s)" or "event(s)" in the success message while maintaining the same pluralization logic.plan.md-90-90 (1)
90-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve markdownlint warnings in this plan document.
- Line 90: fenced block is missing a language tag (
MD040).- Line 981: code span has spacing issue (
MD038).These are quick fixes and keep docs CI/lint clean.
Also applies to: 981-981
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plan.md` at line 90, Fix two markdownlint warnings in the document. For the fenced code block that is currently missing a language tag, add an appropriate language identifier after the opening backticks (for example, if it contains code, use a language like bash, python, javascript, etc.). For the code span with spacing issues, ensure there are no unnecessary spaces between the backticks and the content inside (the space should be removed if present on either side of the text within the backticks).Source: Linters/SAST tools
apps/wodsmith-start/test/lib/organizer-file-import/validate.test.ts-43-309 (1)
43-309:⚠️ Potential issue | 🟡 MinorAdd
@lat:references to each test case.Test specifications exist in
lat.md/architecture.mdunder "Organizer file-drop import" (lines 204–212). Each test case requires exactly one//@lat:comment placed next to the test, per the coding guidelines.Use the section reference:
//@lat: [[architecture#Organizer file-drop import]]Add this comment to each of the 13 test cases across all describe blocks (classifyVolunteer, reconcileVolunteerProposal, isBlockedVolunteer, planVolunteerApply, planEventApply, and validateEventProposal).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/test/lib/organizer-file-import/validate.test.ts` around lines 43 - 309, Add a `// `@lat`: [[architecture#Organizer file-drop import]]` comment to each test case in the file, placed next to the it() function calls. This needs to be done for all test cases across the six describe blocks: classifyVolunteer, reconcileVolunteerProposal, isBlockedVolunteer, planVolunteerApply, planEventApply, and validateEventProposal. The comment should be placed immediately before or at the same line level as each individual test case (it() function) to reference the corresponding architecture documentation section.Source: Coding guidelines
apps/wodsmith-start/src/db/schemas/common.ts-146-147 (1)
146-147:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the required LAT code reference for the new ID generator block.
Please add a
//@lat: [[section-id]]comment adjacent to this new section to keep code-to-concept traceability consistent.As per coding guidelines,
**/*.{js,ts,rs,go,c,h,py}must tie source code to concepts using code refs://@lat: [[section-id]].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/db/schemas/common.ts` around lines 146 - 147, The createAgentImportRunId function is missing the required LAT code reference comment for traceability. Add a // `@lat`: [[section-id]] comment directly above or adjacent to the createAgentImportRunId function definition to maintain consistency with coding guidelines that require all TypeScript source files to include code-to-concept traceability references. Replace section-id with the appropriate concept identifier that corresponds to this ID generator's functionality.Source: Coding guidelines
apps/wodsmith-start/alchemy.run.ts-530-546 (1)
530-546:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd LAT references for the new Durable Object import-agent wiring.
Please add
//@lat: [[section-id]]comments next to the new DO namespace and binding blocks.As per coding guidelines,
**/*.{js,ts,rs,go,c,h,py}must tie source code to concepts using code refs://@lat: [[section-id]].Also applies to: 679-680
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/alchemy.run.ts` around lines 530 - 546, The code is missing LAT (Linked Abstraction Tags) references required by coding guidelines. Add a // `@lat`: [[section-id]] comment above or next to the organizerFileImportAgent DurableObjectNamespace declaration and also at the binding blocks mentioned around lines 679-680. Ensure each DO namespace and binding block has the appropriate LAT reference comment to tie the source code to its corresponding concept section.Source: Coding guidelines
apps/wodsmith-start/src/lib/organizer-file-import/parse.ts-1-12 (1)
1-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd LAT code references for this new parser implementation.
Please add
//@lat: [[section-id]]comments near the key parsing/model-bounding sections in this module.As per coding guidelines,
**/*.{js,ts,rs,go,c,h,py}must tie source code to concepts using code refs://@lat: [[section-id]].Also applies to: 16-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/lib/organizer-file-import/parse.ts` around lines 1 - 12, The parse.ts module is missing required LAT code references that tie source code to concepts per coding guidelines. Add `// `@lat`: [[section-id]]` comments near key parsing and model-bounding sections throughout the module (covering the range from line 1 through line 133). Place these code ref comments at critical points such as before functions that handle different file type parsing (CSV/TSV parsing logic, plain text/markdown parsing logic), at model normalization boundaries, and at the validation sections that match against existing data. Ensure each section-id uniquely identifies the concept or functionality being referenced.Source: Coding guidelines
apps/wodsmith-start/src/server/organizer-file-import/context.ts-26-33 (1)
26-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd LAT references to the new context-loader module.
Please add
//@lat: [[section-id]]comments near the volunteer/event loader and R2 file-read sections.As per coding guidelines,
**/*.{js,ts,rs,go,c,h,py}must tie source code to concepts using code refs://@lat: [[section-id]].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/server/organizer-file-import/context.ts` around lines 26 - 33, The context.ts file is missing required LAT reference comments per coding guidelines. Add `// `@lat`: [[section-id]]` comments to the module, specifically near the volunteer/invites loader function, the existing events loader function, and the R2 file-read section. These comments should appear above or inline with their respective functions or code blocks to tie the source code to architectural concepts. Replace [[section-id]] with the appropriate architectural section identifiers for each context loader.Source: Coding guidelines
apps/wodsmith-start/src/db/schemas/agent-imports.ts-24-37 (1)
24-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd LAT code references in this new schema module.
Please annotate the new route/status and table sections with
//@lat: [[section-id]]comments.As per coding guidelines,
**/*.{js,ts,rs,go,c,h,py}must tie source code to concepts using code refs://@lat: [[section-id]].Also applies to: 75-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/db/schemas/agent-imports.ts` around lines 24 - 37, Add LAT code reference annotations to the new schema sections in the agent-imports.ts file following the coding guidelines. Specifically, add `// `@lat`: [[section-id]]` comments above the route/status and table sections, including the AGENT_IMPORT_ROUTE_KIND object definition, the AgentImportRouteKind type definition, and the AGENT_IMPORT_ROUTE_KIND_VALUES constant. Replace `section-id` with the appropriate LAT concept identifier for each section to tie the source code to its corresponding documentation or design concepts.Source: Coding guidelines
apps/wodsmith-start/test/lib/organizer-file-import/parse.test.ts-14-111 (1)
14-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd exactly one
@latreference next to each test case.Each
it(...)block in this file needs one adjacent//@lat: [[section-id]]comment; none are present right now.As per coding guidelines,
**/{test,tests,spec,specs}/**/*.{js,ts,py,rs,go,c,h}requires exactly one//@lat:/#@lat:per test next to the relevant test, with no duplicates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/test/lib/organizer-file-import/parse.test.ts` around lines 14 - 111, Add exactly one `// `@lat`: [[section-id]]` comment adjacent to each `it(...)` test block in the file to comply with coding guidelines. The test blocks that need these comments are: parseDelimited with "parses CSV into header-keyed rows", "trims headers and skips empty lines", and "parses TSV when given a tab delimiter"; parseImportFile with "parses by csv mime type", "falls back to extension when mime is generic", "returns text for plain text / markdown", and "throws on unsupported types"; boundTableForModel with "caps rows and adds a warning" and "leaves small tables untouched"; and renderParsedForModel with "renders columns and rows compactly". Place each comment immediately before or after its corresponding `it(...)` block, ensure no duplicate section-ids are used, and maintain consistency across all test cases.Source: Coding guidelines
apps/wodsmith-start/src/server/organizer-file-import/access.ts-21-31 (1)
21-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd LAT code references for this authorization module.
Please add
//@lat: [[section-id]]comments near the scope-loading and access-check sections.As per coding guidelines,
**/*.{js,ts,rs,go,c,h,py}must tie source code to concepts using code refs://@lat: [[section-id]].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/server/organizer-file-import/access.ts` around lines 21 - 31, The access authorization module for organizer file import is missing LAT code reference comments required by coding guidelines. Add `// `@lat`: [[section-id]]` comments to tie the source code concepts to the LAT system. Specifically, add these comments near the scope-loading section for requireFileImportTeamAccess (the request-context path using Start cookie helpers and requireTeamPermission) and the access-check section for requireFileImportAgentAccess (the Durable-Object path using DB-direct access), ensuring both enforcement points for competition ownership, MANAGE_COMPETITIONS permission, and AI_FILE_IMPORT entitlement are properly referenced.Source: Coding guidelines
🧹 Nitpick comments (2)
apps/wodsmith-start/src/server.ts (1)
148-165: 💤 Low valueConsider extracting shared DO routing logic.
The routing logic for
organizer-file-import-agent(lines 148-165) is nearly identical tojudge-scheduler-agent(lines 124-147). As more agents are added, this pattern could be extracted into a helper function:async function routeToUserScopedAgent<T>( request: Request, name: string, ns: DurableObjectNamespace<T>, ): Promise<Response | null> { const match = /^([a-z0-9_-]{1,128})__([a-z0-9_-]{1,128})$/i.exec(name) if (!match) return new Response("Invalid agent name", { status: 400 }) const [, , userId] = match const session = await getSessionFromRequestCookie(request) if (!session?.userId || session.userId !== userId) { return new Response("Unauthorized", { status: 401 }) } const stub = await getAgentByName(ns, name) return stub.fetch(request) }This would reduce the per-agent routing to selecting the correct namespace binding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/server.ts` around lines 148 - 165, Extract the shared routing logic that is duplicated between the organizer-file-import-agent and judge-scheduler-agent handlers into a reusable generic helper function. Create a helper function that accepts the request, agent name, and DurableObjectNamespace as parameters, and handles the name validation with the regex pattern, session verification with getSessionFromRequestCookie, userId comparison, agent stub retrieval via getAgentByName, and the final fetch call. This will allow both agent handlers to simply call this helper function instead of repeating the same logic, and makes it easier to add new agents in the future without duplicating this pattern.apps/wodsmith-start/src/lib/organizer-file-import/parse.ts (1)
23-35: ⚡ Quick winUse interfaces for object-shape contracts in this TS module.
ParsedTableandParsedTextare object-shape aliases and should be interfaces per project TypeScript conventions.As per coding guidelines,
**/*.{ts,tsx}should use TypeScript everywhere and prefer interfaces over types.Suggested refactor
-export type ParsedTable = { +export interface ParsedTable { kind: "table" headers: string[] rows: Record<string, string>[] rowCount: number warnings: string[] } -export type ParsedText = { +export interface ParsedText { kind: "text" text: string warnings: string[] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/lib/organizer-file-import/parse.ts` around lines 23 - 35, Convert the ParsedTable and ParsedText type aliases into interface declarations to align with project TypeScript conventions. Replace the export type keyword with export interface for both ParsedTable and ParsedText, maintaining the same property structure and removing the equals sign from the syntax. This ensures consistency with the project's preference for interfaces over type aliases for object-shape contracts.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx`:
- Around line 279-303: The refine input flow currently uses manual useState
state management (refineText) and ad-hoc validation with trim checks instead of
following the project's form standard. Replace the useState for refineText with
React Hook Form by creating a useForm hook with a Zod schema that validates the
refine input as a non-empty string, then update the Textarea component to use
the form's register method instead of the onChange handler, and modify the
handleRefine function to work with React Hook Form's handleSubmit pattern.
Remove the manual refineText.trim() validation check since Zod will handle
validation automatically.
In `@apps/wodsmith-start/src/components/organizer-import/import-shell.tsx`:
- Around line 176-183: The ImportReviewDrawer component is missing a key prop
and is being reused when a new run is selected, which causes stale internal
state to persist across different import runs. Add a key prop to the
ImportReviewDrawer component that uses the importRunId value passed to it (the
key should be set to the importRunId or run.importRunId). This will ensure that
React properly unmounts the old drawer instance and mounts a fresh one whenever
a different importRunId is selected, clearing any stale internal state and refs
from the previous run.
In `@apps/wodsmith-start/src/components/organizer-import/use-page-intent.ts`:
- Around line 1-3: The file use-page-intent.ts is missing the required `@lat` code
reference comment that must be present in all TypeScript files per coding
guidelines. Add a comment in the format // `@lat`: [[section-id]] after the import
statements near the top of the file, replacing section-id with the appropriate
concept identifier that corresponds to this component's functionality.
- Around line 21-33: The route matching condition in the function is too broad
because it uses includes("/volunteers") which matches any route containing that
substring, including /volunteers/shifts and /volunteers/signup-questions that
should not trigger the import intent. Replace the includes("/volunteers") check
with a more specific pattern that only matches the intended Volunteers roster
page, either by checking if the routeId equals "/volunteers" exactly or ends
with "/volunteers" without additional path segments after it, similar to how the
judges route check is already specific to "/volunteers/judges".
In `@apps/wodsmith-start/src/db/schemas/agent-imports.ts`:
- Around line 112-115: The agent_import_runs_checksum_idx index in the
agent-imports schema is non-unique, which allows duplicate imports for the same
competitionId and checksum to be inserted concurrently, breaking idempotency
guarantees. Replace the index() method call with unique() for the same column
combination (table.competitionId and table.checksum) to enforce a unique
constraint that prevents duplicate imports at the database level.
In `@apps/wodsmith-start/src/routes/api/agent-import/upload.ts`:
- Around line 110-127: The `key` variable is being logged in both the logInfo
and logError calls but should be removed because it contains the filename which
can include PII. In the logInfo attributes object (which logs the successful
upload completion), remove `key` from the attributes. Similarly, in the logError
attributes object (which logs the R2 upload failure in the catch block), remove
`key` from the attributes. The `key` can remain in the json response return
value since that is the API response, not a log output.
- Around line 63-69: The calls to loadFileImportScopeByRun and
requireFileImportTeamAccess are not handling errors thrown for missing runs,
wrong team, or permission issues, causing them to result in 500 status codes
instead of proper 404/403 responses. Wrap these function calls in a try-catch
block and map the thrown errors to appropriate HTTP status codes: return 404 for
not found errors and 403 for permission or access denied errors. Ensure the
error handling is consistent with other API routes in the `/routes/api`
directory by catching specific error types and returning the correct response
status.
- Around line 97-108: The database update operation on agentImportRunsTable
needs to guard against state transitions to prevent stale or replayed uploads
from moving already-processed runs back to UPLOADED status. Add a status
condition to the WHERE clause to only allow the update when the current status
is CREATED (using eq with the status field and AGENT_IMPORT_STATUS.CREATED).
After executing the update, capture the result and check the affected rows
count. If zero rows were affected, it means the run was not in the expected
CREATED state, so you should handle this case (e.g., throw an error or return a
failure response) before treating the upload as successful. Only proceed with
success handling if the update affected exactly one row.
- Around line 44-50: The current code casts form.get("file") directly to File |
null without validating that it is actually a File instance, since form.get()
can return either a string or a File. Add a type check using instanceof File to
ensure the file is actually a File object before using properties like size,
type, or name. If the file is not a File instance, it should be treated as
invalid and the existing 400 error response should be returned. Update the
validation logic in the conditional block (the if statement checking for !file
|| !importRunId) to include an instanceof File check that will properly reject
malformed requests with a 400 status instead of allowing them to reach file
property access and return a 500 error.
In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts`:
- Around line 144-150: The apply operation does not validate the status of the
run before proceeding with the apply logic. After loading the file import scope
with loadFileImportScopeByRun and checking team access, add validation to ensure
scope.run.status is in an applicable state (not in terminal states like CREATED,
APPLIED, or REJECTED) before calling getDb() and executing any database writes.
If the status is invalid, throw an appropriate error to reject the operation.
- Around line 263-270: The delete statement for teamInvitationTable only filters
by id, which creates a race condition where an invite accepted between the read
check and delete can still be deleted. Modify the delete WHERE clause to include
both the id check AND the acceptedAt IS NULL condition to ensure only pending
invites are deleted. Additionally, capture the number of affected rows from the
delete operation and only increment skippedCount when the delete actually
affected a row, ensuring accurate tracking of what was truly removed.
- Around line 296-301: The update to agentImportRunsTable in the REJECTED status
block unconditionally clears appliedEntities by setting it to an empty JSON
array, which loses the audit trail of skipped entities. When incrementing
skippedCount during undo operations, the skipped entities should be preserved in
the appliedEntities field instead of being overwritten with an empty array.
Modify the update logic to preserve or append skipped entities to the
appliedEntities field rather than clearing it, ensuring the receipt list
maintains the complete history for potential retries.
- Around line 178-220: The current implementation creates volunteers and events
(via executeVolunteerInvite and executeEventCreate) and accumulates the results
in appliedEntities before finally persisting the import run record via db.update
on agentImportRunsTable. If the final database update fails or the request
aborts after creating resources but before persisting the run metadata, the
system loses track of applied entities, breaking idempotency and undo
capabilities. Wrap the entire workflow from loading existing data through the
final db.update call in a database transaction using db.transaction to ensure
all writes are atomic together, so that either all changes commit together or
none do, providing a clear recovery path for retries.
In `@apps/wodsmith-start/src/server/organizer-file-import/access.ts`:
- Around line 50-54: The loadFileImportScope function accepts arbitrary
routeKind values and allows inconsistent combinations between routeKind and
eventId parameters without validation. Add validation guards at the beginning of
the loadFileImportScope function to enforce that routeKind only accepts valid
values, require eventId to be present when routeKind is event_detail, and reject
cases where eventId is provided for non-event_detail routes. This validation
should occur before the scope is resolved to prevent invalid states from
propagating to downstream agent and apply paths.
In `@apps/wodsmith-start/src/server/organizer-file-import/context.ts`:
- Around line 72-76: The filter conditions in the and() function for volunteer
team memberships need to be updated to exclude inactive records. Add an
additional condition to the and() function that checks for active status (likely
using a field like isActive or status on the teamMembershipTable) alongside the
existing checks for teamId, roleId, and isSystemRole to ensure only active
volunteer memberships are included in the existing volunteer context query.
---
Minor comments:
In `@apps/wodsmith-start/alchemy.run.ts`:
- Around line 530-546: The code is missing LAT (Linked Abstraction Tags)
references required by coding guidelines. Add a // `@lat`: [[section-id]] comment
above or next to the organizerFileImportAgent DurableObjectNamespace declaration
and also at the binding blocks mentioned around lines 679-680. Ensure each DO
namespace and binding block has the appropriate LAT reference comment to tie the
source code to its corresponding concept section.
In
`@apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx`:
- Around line 164-166: The toast success message in the undo operation always
displays "invitation(s)" regardless of what was actually undone, but the
operation can revert either invitations or events. Update the toast.success call
to dynamically construct the message based on what type of objects were undone.
Check the result object for additional properties that indicate whether
invitations or events were reverted, and use that information to display either
"invitation(s)" or "event(s)" in the success message while maintaining the same
pluralization logic.
In `@apps/wodsmith-start/src/db/schemas/agent-imports.ts`:
- Around line 24-37: Add LAT code reference annotations to the new schema
sections in the agent-imports.ts file following the coding guidelines.
Specifically, add `// `@lat`: [[section-id]]` comments above the route/status and
table sections, including the AGENT_IMPORT_ROUTE_KIND object definition, the
AgentImportRouteKind type definition, and the AGENT_IMPORT_ROUTE_KIND_VALUES
constant. Replace `section-id` with the appropriate LAT concept identifier for
each section to tie the source code to its corresponding documentation or design
concepts.
In `@apps/wodsmith-start/src/db/schemas/common.ts`:
- Around line 146-147: The createAgentImportRunId function is missing the
required LAT code reference comment for traceability. Add a // `@lat`:
[[section-id]] comment directly above or adjacent to the createAgentImportRunId
function definition to maintain consistency with coding guidelines that require
all TypeScript source files to include code-to-concept traceability references.
Replace section-id with the appropriate concept identifier that corresponds to
this ID generator's functionality.
In `@apps/wodsmith-start/src/lib/organizer-file-import/parse.ts`:
- Around line 1-12: The parse.ts module is missing required LAT code references
that tie source code to concepts per coding guidelines. Add `// `@lat`:
[[section-id]]` comments near key parsing and model-bounding sections throughout
the module (covering the range from line 1 through line 133). Place these code
ref comments at critical points such as before functions that handle different
file type parsing (CSV/TSV parsing logic, plain text/markdown parsing logic), at
model normalization boundaries, and at the validation sections that match
against existing data. Ensure each section-id uniquely identifies the concept or
functionality being referenced.
In `@apps/wodsmith-start/src/routes/api/agent-import/upload.ts`:
- Around line 1-8: The TypeScript source file for the upload endpoint is missing
the required `// `@lat`: [[section-id]]` code reference that ties this module to
the organizer import concept. Add a `// `@lat`: [[...]]` comment near the module
header or route definition (within the first few lines after any imports) using
the appropriate section id that corresponds to the organizer import
functionality. This reference is required by coding guidelines for all source
files to establish traceability to architectural concepts.
In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts`:
- Around line 1-10: The organizer-file-import-fns.ts module is missing the
required code reference annotation. Add a `// `@lat`: [[section-id]]` comment near
the module header (after the existing JSDoc block) or adjacent to the exported
server functions like createImportRunFn and loadFileImportContextFn to tie this
write path to the organizer import spec. Replace section-id with the actual
section identifier from the organizer import specification as per the coding
guidelines for source-to-concept traceability.
- Around line 407-414: The createWorkoutAndAddToCompetitionFn call is missing
the scoreType parameter in its data object. Add scoreType from the decision
object to the data parameter passed to createWorkoutAndAddToCompetitionFn, using
a nullish coalescing operator to handle cases where scoreType may be undefined,
ensuring the imported workout's scoring configuration is properly forwarded.
In `@apps/wodsmith-start/src/server/organizer-file-import/access.ts`:
- Around line 21-31: The access authorization module for organizer file import
is missing LAT code reference comments required by coding guidelines. Add `//
`@lat`: [[section-id]]` comments to tie the source code concepts to the LAT
system. Specifically, add these comments near the scope-loading section for
requireFileImportTeamAccess (the request-context path using Start cookie helpers
and requireTeamPermission) and the access-check section for
requireFileImportAgentAccess (the Durable-Object path using DB-direct access),
ensuring both enforcement points for competition ownership, MANAGE_COMPETITIONS
permission, and AI_FILE_IMPORT entitlement are properly referenced.
In `@apps/wodsmith-start/src/server/organizer-file-import/context.ts`:
- Around line 26-33: The context.ts file is missing required LAT reference
comments per coding guidelines. Add `// `@lat`: [[section-id]]` comments to the
module, specifically near the volunteer/invites loader function, the existing
events loader function, and the R2 file-read section. These comments should
appear above or inline with their respective functions or code blocks to tie the
source code to architectural concepts. Replace [[section-id]] with the
appropriate architectural section identifiers for each context loader.
In `@apps/wodsmith-start/test/lib/organizer-file-import/parse.test.ts`:
- Around line 14-111: Add exactly one `// `@lat`: [[section-id]]` comment adjacent
to each `it(...)` test block in the file to comply with coding guidelines. The
test blocks that need these comments are: parseDelimited with "parses CSV into
header-keyed rows", "trims headers and skips empty lines", and "parses TSV when
given a tab delimiter"; parseImportFile with "parses by csv mime type", "falls
back to extension when mime is generic", "returns text for plain text /
markdown", and "throws on unsupported types"; boundTableForModel with "caps rows
and adds a warning" and "leaves small tables untouched"; and
renderParsedForModel with "renders columns and rows compactly". Place each
comment immediately before or after its corresponding `it(...)` block, ensure no
duplicate section-ids are used, and maintain consistency across all test cases.
In `@apps/wodsmith-start/test/lib/organizer-file-import/validate.test.ts`:
- Around line 43-309: Add a `// `@lat`: [[architecture#Organizer file-drop
import]]` comment to each test case in the file, placed next to the it()
function calls. This needs to be done for all test cases across the six describe
blocks: classifyVolunteer, reconcileVolunteerProposal, isBlockedVolunteer,
planVolunteerApply, planEventApply, and validateEventProposal. The comment
should be placed immediately before or at the same line level as each individual
test case (it() function) to reference the corresponding architecture
documentation section.
In `@plan.md`:
- Line 90: Fix two markdownlint warnings in the document. For the fenced code
block that is currently missing a language tag, add an appropriate language
identifier after the opening backticks (for example, if it contains code, use a
language like bash, python, javascript, etc.). For the code span with spacing
issues, ensure there are no unnecessary spaces between the backticks and the
content inside (the space should be removed if present on either side of the
text within the backticks).
---
Nitpick comments:
In `@apps/wodsmith-start/src/lib/organizer-file-import/parse.ts`:
- Around line 23-35: Convert the ParsedTable and ParsedText type aliases into
interface declarations to align with project TypeScript conventions. Replace the
export type keyword with export interface for both ParsedTable and ParsedText,
maintaining the same property structure and removing the equals sign from the
syntax. This ensures consistency with the project's preference for interfaces
over type aliases for object-shape contracts.
In `@apps/wodsmith-start/src/server.ts`:
- Around line 148-165: Extract the shared routing logic that is duplicated
between the organizer-file-import-agent and judge-scheduler-agent handlers into
a reusable generic helper function. Create a helper function that accepts the
request, agent name, and DurableObjectNamespace as parameters, and handles the
name validation with the regex pattern, session verification with
getSessionFromRequestCookie, userId comparison, agent stub retrieval via
getAgentByName, and the final fetch call. This will allow both agent handlers to
simply call this helper function instead of repeating the same logic, and makes
it easier to add new agents in the future without duplicating this pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df3390b2-df7a-46ad-ba54-73d66d80eb61
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
apps/wodsmith-start/alchemy.run.tsapps/wodsmith-start/package.jsonapps/wodsmith-start/src/agents/organizer-file-import-agent.tsapps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsxapps/wodsmith-start/src/components/organizer-import/import-shell.tsxapps/wodsmith-start/src/components/organizer-import/use-page-intent.tsapps/wodsmith-start/src/config/features.tsapps/wodsmith-start/src/db/schema.tsapps/wodsmith-start/src/db/schemas/agent-imports.tsapps/wodsmith-start/src/db/schemas/common.tsapps/wodsmith-start/src/lib/organizer-file-import/parse.tsapps/wodsmith-start/src/lib/organizer-file-import/schemas.tsapps/wodsmith-start/src/lib/organizer-file-import/validate.tsapps/wodsmith-start/src/routeTree.gen.tsapps/wodsmith-start/src/routes/api/agent-import/upload.tsapps/wodsmith-start/src/routes/compete/organizer/$competitionId.tsxapps/wodsmith-start/src/server-fns/organizer-file-import-fns.tsapps/wodsmith-start/src/server.tsapps/wodsmith-start/src/server/organizer-file-import/access.tsapps/wodsmith-start/src/server/organizer-file-import/context.tsapps/wodsmith-start/src/types/env.d.tsapps/wodsmith-start/test/lib/organizer-file-import/parse.test.tsapps/wodsmith-start/test/lib/organizer-file-import/validate.test.tslat.md/architecture.mdlat.md/organizer-dashboard.mdplan.md
Comment on lines
+279
to
+303
| <div className="space-y-2"> | ||
| <Textarea | ||
| value={refineText} | ||
| onChange={(e) => setRefineText(e.target.value)} | ||
| placeholder={ | ||
| isEventMode | ||
| ? "Refine in words — e.g. “skip the warm-up, set the AMRAP to 20 minutes”" | ||
| : "Refine in words — e.g. “make the coaches head judges, skip anyone without an email”" | ||
| } | ||
| rows={2} | ||
| disabled={isWorking || isRefining} | ||
| /> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={handleRefine} | ||
| disabled={isWorking || isRefining || refineText.trim() === ""} | ||
| > | ||
| {isRefining ? ( | ||
| <Loader2 className="mr-2 h-4 w-4 animate-spin" /> | ||
| ) : ( | ||
| <RotateCcw className="mr-2 h-4 w-4" /> | ||
| )} | ||
| Refine draft | ||
| </Button> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use React Hook Form + Zod for the refine input flow.
This refine control is a form interaction but currently uses ad-hoc useState + trim checks instead of the project’s form standard.
As per coding guidelines, **/*.tsx should use React Hook Form with Zod validation for forms.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/components/organizer-import/import-review-drawer.tsx`
around lines 279 - 303, The refine input flow currently uses manual useState
state management (refineText) and ad-hoc validation with trim checks instead of
following the project's form standard. Replace the useState for refineText with
React Hook Form by creating a useForm hook with a Zod schema that validates the
refine input as a non-empty string, then update the Textarea component to use
the form's register method instead of the onChange handler, and modify the
handleRefine function to work with React Hook Form's handleSubmit pattern.
Remove the manual refineText.trim() validation check since Zod will handle
validation automatically.
Source: Coding guidelines
Comment on lines
+176
to
+183
| {run && ( | ||
| <ImportReviewDrawer | ||
| importRunId={run.importRunId} | ||
| competitionId={competition.id} | ||
| routeKind={run.intent.routeKind} | ||
| eventId={run.intent.eventId} | ||
| onClose={() => setRun(null)} | ||
| /> |
Contributor
There was a problem hiding this comment.
Drawer instance is not keyed by importRunId, so a new run can reuse stale internal state.
When a second file upload updates run, ImportReviewDrawer is reused instead of remounted. Because the drawer keeps one-time startup refs, the new run can fail to initialize correctly.
💡 Suggested fix
{run && (
<ImportReviewDrawer
+ key={run.importRunId}
importRunId={run.importRunId}
competitionId={competition.id}
routeKind={run.intent.routeKind}
eventId={run.intent.eventId}
onClose={() => setRun(null)}
/>
)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/components/organizer-import/import-shell.tsx` around
lines 176 - 183, The ImportReviewDrawer component is missing a key prop and is
being reused when a new run is selected, which causes stale internal state to
persist across different import runs. Add a key prop to the ImportReviewDrawer
component that uses the importRunId value passed to it (the key should be set to
the importRunId or run.importRunId). This will ensure that React properly
unmounts the old drawer instance and mounts a fresh one whenever a different
importRunId is selected, clearing any stale internal state and refs from the
previous run.
Comment on lines
+1
to
+3
| import { useMatches } from "@tanstack/react-router" | ||
| import type { AgentImportRouteKind } from "@/db/schemas/agent-imports" | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add the required @lat code reference to this TypeScript file.
This file is missing the required concept link comment (e.g., near the header imports).
As per coding guidelines, **/*.{js,ts,rs,go,c,h,py} must tie source code to concepts using // @lat: [[section-id]].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/components/organizer-import/use-page-intent.ts`
around lines 1 - 3, The file use-page-intent.ts is missing the required `@lat`
code reference comment that must be present in all TypeScript files per coding
guidelines. Add a comment in the format // `@lat`: [[section-id]] after the import
statements near the top of the file, replacing section-id with the appropriate
concept identifier that corresponds to this component's functionality.
Source: Coding guidelines
Comment on lines
+112
to
+115
| index("agent_import_runs_checksum_idx").on( | ||
| table.competitionId, | ||
| table.checksum, | ||
| ), |
Contributor
There was a problem hiding this comment.
Enforce duplicate-import protection with a unique constraint.
The checksum index is non-unique, so concurrent inserts can still create duplicate runs for the same (competitionId, checksum). That breaks idempotency under race conditions.
Suggested schema change
import {
datetime,
index,
int,
mysqlTable,
text,
+ uniqueIndex,
varchar,
} from "drizzle-orm/mysql-core"
@@
- index("agent_import_runs_checksum_idx").on(
+ uniqueIndex("agent_import_runs_competition_checksum_uidx").on(
table.competitionId,
table.checksum,
),📝 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
| index("agent_import_runs_checksum_idx").on( | |
| table.competitionId, | |
| table.checksum, | |
| ), | |
| uniqueIndex("agent_import_runs_competition_checksum_uidx").on( | |
| table.competitionId, | |
| table.checksum, | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/db/schemas/agent-imports.ts` around lines 112 - 115,
The agent_import_runs_checksum_idx index in the agent-imports schema is
non-unique, which allows duplicate imports for the same competitionId and
checksum to be inserted concurrently, breaking idempotency guarantees. Replace
the index() method call with unique() for the same column combination
(table.competitionId and table.checksum) to enforce a unique constraint that
prevents duplicate imports at the database level.
Comment on lines
+178
to
+220
| const outcome = await executeVolunteerInvite(decision, scope) | ||
| results.push(outcome.result) | ||
| if (outcome.entity) appliedEntities.push(outcome.entity) | ||
| } | ||
|
|
||
| if (data.eventProposals.length > 0) { | ||
| const existingEvents = await loadExistingEvents(scope.competitionId) | ||
| const eventDecisions = planEventApply(data.eventProposals, { | ||
| alreadyAppliedRowKeys: alreadyApplied, | ||
| existingEvents, | ||
| allowedSchemes: WORKOUT_SCHEME_VALUES, | ||
| }) | ||
| for (const decision of eventDecisions) { | ||
| if (decision.outcome === "skip") { | ||
| results.push( | ||
| skipped(decision.rowKey, "event_create", decision.reason), | ||
| ) | ||
| continue | ||
| } | ||
| if (decision.outcome === "fail") { | ||
| results.push(failed(decision.rowKey, "event_create", decision.reason)) | ||
| continue | ||
| } | ||
| const outcome = await executeEventCreate(decision, scope) | ||
| results.push(outcome.result) | ||
| if (outcome.entity) appliedEntities.push(outcome.entity) | ||
| } | ||
| } | ||
|
|
||
| const appliedCount = results.filter((r) => r.status === "applied").length | ||
| const skippedCount = results.filter((r) => r.status === "skipped").length | ||
| const failedCount = results.filter((r) => r.status === "failed").length | ||
|
|
||
| const session = await getSessionFromCookie() | ||
| await db | ||
| .update(agentImportRunsTable) | ||
| .set({ | ||
| status: AGENT_IMPORT_STATUS.APPLIED, | ||
| appliedEntities: JSON.stringify(appliedEntities), | ||
| appliedByUserId: session?.user.id ?? scope.createdByUserId, | ||
| appliedAt: new Date(), | ||
| }) | ||
| .where(eq(agentImportRunsTable.id, data.importRunId)) |
Contributor
There was a problem hiding this comment.
Persist receipts atomically with the writes that need undo.
Invites/events are created before appliedEntities is saved; if the final run update fails or the request aborts after some writes, the system loses the IDs needed for idempotency and undo. Wrap DB-only work in a transaction or persist each successful receipt before continuing, with a clear recovery path for external calls. Based on learnings, this repo uses PlanetScale/MySQL and multi-step writes should use db.transaction for atomic boundaries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts` around lines
178 - 220, The current implementation creates volunteers and events (via
executeVolunteerInvite and executeEventCreate) and accumulates the results in
appliedEntities before finally persisting the import run record via db.update on
agentImportRunsTable. If the final database update fails or the request aborts
after creating resources but before persisting the run metadata, the system
loses track of applied entities, breaking idempotency and undo capabilities.
Wrap the entire workflow from loading existing data through the final db.update
call in a database transaction using db.transaction to ensure all writes are
atomic together, so that either all changes commit together or none do,
providing a clear recovery path for retries.
Source: Learnings
Comment on lines
+263
to
+270
| if (!invite || invite.acceptedAt) { | ||
| // Already accepted (now a membership) or already gone — leave it. | ||
| skippedCount++ | ||
| continue | ||
| } | ||
| await db | ||
| .delete(teamInvitationTable) | ||
| .where(eq(teamInvitationTable.id, entity.entityId)) |
Contributor
There was a problem hiding this comment.
Make the pending-invite delete conditional.
The read checks acceptedAt, but the delete only filters by id; an invite accepted between those statements can still be deleted despite the “only if still pending” rule. Delete with both id and acceptedAt IS NULL, then count it undone only when a row was affected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts` around lines
263 - 270, The delete statement for teamInvitationTable only filters by id,
which creates a race condition where an invite accepted between the read check
and delete can still be deleted. Modify the delete WHERE clause to include both
the id check AND the acceptedAt IS NULL condition to ensure only pending invites
are deleted. Additionally, capture the number of affected rows from the delete
operation and only increment skippedCount when the delete actually affected a
row, ensuring accurate tracking of what was truly removed.
Comment on lines
+296
to
+301
| await db | ||
| .update(agentImportRunsTable) | ||
| .set({ | ||
| status: AGENT_IMPORT_STATUS.REJECTED, | ||
| appliedEntities: JSON.stringify([]), | ||
| }) |
Contributor
There was a problem hiding this comment.
Do not clear receipts for entities that were skipped during undo.
When an invite is accepted or event removal fails, skippedCount is incremented, but appliedEntities is still overwritten with []. That loses the audit trail and prevents retrying failed event undos; keep skipped entities in the stored receipt list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts` around lines
296 - 301, The update to agentImportRunsTable in the REJECTED status block
unconditionally clears appliedEntities by setting it to an empty JSON array,
which loses the audit trail of skipped entities. When incrementing skippedCount
during undo operations, the skipped entities should be preserved in the
appliedEntities field instead of being overwritten with an empty array. Modify
the update logic to preserve or append skipped entities to the appliedEntities
field rather than clearing it, ensuring the receipt list maintains the complete
history for potential retries.
Comment on lines
+50
to
+54
| export async function loadFileImportScope(input: { | ||
| competitionId: string | ||
| routeKind: string | ||
| eventId?: string | null | ||
| }): Promise<FileImportScope> { |
Contributor
There was a problem hiding this comment.
Validate routeKind and enforce eventId invariants at scope resolution.
This layer currently allows inconsistent combinations (e.g., non-event_detail routes with eventId, or event_detail without eventId) and accepts arbitrary routeKind strings. Enforce these invariants here to prevent invalid scopes from flowing into agent/apply paths.
Suggested guard pattern
+import {
+ AGENT_IMPORT_ROUTE_KIND,
+ AGENT_IMPORT_ROUTE_KIND_VALUES,
+ type AgentImportRouteKind,
+} from "`@/db/schemas/agent-imports`"
@@
export async function loadFileImportScope(input: {
competitionId: string
- routeKind: string
+ routeKind: string
eventId?: string | null
}): Promise<FileImportScope> {
+ if (
+ !AGENT_IMPORT_ROUTE_KIND_VALUES.includes(
+ input.routeKind as AgentImportRouteKind,
+ )
+ ) {
+ throw new Error("Unsupported import route")
+ }
+
+ const isEventDetail = input.routeKind === AGENT_IMPORT_ROUTE_KIND.EVENT_DETAIL
+ if (isEventDetail && !input.eventId) {
+ throw new Error("eventId is required for event_detail imports")
+ }
+ if (!isEventDetail && input.eventId) {
+ throw new Error("eventId is only allowed for event_detail imports")
+ }Also applies to: 72-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/server/organizer-file-import/access.ts` around lines
50 - 54, The loadFileImportScope function accepts arbitrary routeKind values and
allows inconsistent combinations between routeKind and eventId parameters
without validation. Add validation guards at the beginning of the
loadFileImportScope function to enforce that routeKind only accepts valid
values, require eventId to be present when routeKind is event_detail, and reject
cases where eventId is provided for non-event_detail routes. This validation
should occur before the scope is resolved to prevent invalid states from
propagating to downstream agent and apply paths.
Comment on lines
+72
to
+76
| and( | ||
| eq(teamMembershipTable.teamId, competitionTeamId), | ||
| eq(teamMembershipTable.roleId, SYSTEM_ROLES_ENUM.VOLUNTEER), | ||
| eq(teamMembershipTable.isSystemRole, true), | ||
| ), |
Contributor
There was a problem hiding this comment.
Filter volunteer memberships to active records only.
The “existing volunteer” context currently includes inactive memberships, which can incorrectly block or distort dedup/import decisions.
Suggested query fix
.where(
and(
eq(teamMembershipTable.teamId, competitionTeamId),
eq(teamMembershipTable.roleId, SYSTEM_ROLES_ENUM.VOLUNTEER),
eq(teamMembershipTable.isSystemRole, true),
+ eq(teamMembershipTable.isActive, true),
),
)📝 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
| and( | |
| eq(teamMembershipTable.teamId, competitionTeamId), | |
| eq(teamMembershipTable.roleId, SYSTEM_ROLES_ENUM.VOLUNTEER), | |
| eq(teamMembershipTable.isSystemRole, true), | |
| ), | |
| and( | |
| eq(teamMembershipTable.teamId, competitionTeamId), | |
| eq(teamMembershipTable.roleId, SYSTEM_ROLES_ENUM.VOLUNTEER), | |
| eq(teamMembershipTable.isSystemRole, true), | |
| eq(teamMembershipTable.isActive, true), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/src/server/organizer-file-import/context.ts` around lines
72 - 76, The filter conditions in the and() function for volunteer team
memberships need to be updated to exclude inactive records. Add an additional
condition to the and() function that checks for active status (likely using a
field like isActive or status on the teamMembershipTable) alongside the existing
checks for teamId, roleId, and isSystemRole to ensure only active volunteer
memberships are included in the existing volunteer context query.
Boundary-mocked integration tests for the only untested code (the server-fn IO orchestration), keeping the real pure planners in play: - apply records the created invitation, is idempotent on re-apply, fails no-email rows without inviting, and records created events for undo - undo deletes still-pending invitations, leaves accepted ones, removes created events 7 tests (41 total for the feature). Type-checks clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts (1)
69-69: ⚡ Quick winPrefer an interface for
Handlerin this TypeScript file.Line 69 uses a
typealias where repository guidance prefers interfaces in.ts/.tsxfiles.Suggested patch
-type Handler<T> = (args: { data: unknown }) => Promise<T> +interface Handler<T> { + (args: { data: unknown }): Promise<T> +}As per coding guidelines, TypeScript code should prefer interfaces over types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts` at line 69, The Handler<T> type alias should be converted to an interface to comply with TypeScript coding guidelines that prefer interfaces over type aliases in .ts and .tsx files. Replace the type alias declaration with an interface declaration that maintains the same generic parameter and function signature structure, ensuring the interface properly defines the function call signature with the args parameter and Promise return type.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts`:
- Around line 159-287: Each test block in the file is missing the required `@lat`
reference comment. Add a single line comment in the format // `@lat`:
[[section-id]] immediately before or next to each it(...) test case, including
the tests in the applyFn describe block (such as "invites a new volunteer and
records the created invitation", "skips a row already written by a prior apply
(idempotent)", "fails a create with no email and never invites", and "creates an
event and records it for undo") and the tests in the undoImportFn describe block
(such as "deletes a still-pending created invitation", "leaves an
already-accepted invitation alone", and "removes a created event"). Use
appropriate section identifiers for each test and ensure no duplicate `@lat`
references are used across tests.
---
Nitpick comments:
In `@apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts`:
- Line 69: The Handler<T> type alias should be converted to an interface to
comply with TypeScript coding guidelines that prefer interfaces over type
aliases in .ts and .tsx files. Replace the type alias declaration with an
interface declaration that maintains the same generic parameter and function
signature structure, ensuring the interface properly defines the function call
signature with the args parameter and Promise return type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4cc5c68b-2824-445b-9e99-63d56e159426
📒 Files selected for processing (1)
apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts
Comment on lines
+159
to
+287
| it("invites a new volunteer and records the created invitation", async () => { | ||
| h.loadScopeByRun.mockResolvedValue(scopeWith(null)) | ||
| mockDb.setMockSingleValue({ id: "tinv_new" }) // id-capture findFirst | ||
|
|
||
| const result = await applyFn({ | ||
| data: { | ||
| importRunId: "aimp_1", | ||
| volunteerProposals: [volunteerProposal()], | ||
| eventProposals: [], | ||
| }, | ||
| }) | ||
|
|
||
| expect(h.inviteVolunteer).toHaveBeenCalledTimes(1) | ||
| expect(result.appliedCount).toBe(1) | ||
| expect(result.results[0]).toMatchObject({ | ||
| rowKey: "r1", | ||
| status: "applied", | ||
| entityId: "tinv_new", | ||
| }) | ||
| expect(recordedEntities()).toEqual([ | ||
| { kind: "volunteer_invite", entityId: "tinv_new", rowKey: "r1" }, | ||
| ]) | ||
| }) | ||
|
|
||
| it("skips a row already written by a prior apply (idempotent)", async () => { | ||
| const prior = JSON.stringify([ | ||
| { kind: "volunteer_invite", entityId: "tinv_old", rowKey: "r1" }, | ||
| ]) | ||
| h.loadScopeByRun.mockResolvedValue(scopeWith(prior)) | ||
|
|
||
| const result = await applyFn({ | ||
| data: { | ||
| importRunId: "aimp_1", | ||
| volunteerProposals: [volunteerProposal({ rowKey: "r1" })], | ||
| eventProposals: [], | ||
| }, | ||
| }) | ||
|
|
||
| expect(h.inviteVolunteer).not.toHaveBeenCalled() | ||
| expect(result.skippedCount).toBe(1) | ||
| expect(result.results[0]).toMatchObject({ rowKey: "r1", status: "skipped" }) | ||
| }) | ||
|
|
||
| it("fails a create with no email and never invites", async () => { | ||
| h.loadScopeByRun.mockResolvedValue(scopeWith(null)) | ||
|
|
||
| const result = await applyFn({ | ||
| data: { | ||
| importRunId: "aimp_1", | ||
| volunteerProposals: [volunteerProposal({ email: null })], | ||
| eventProposals: [], | ||
| }, | ||
| }) | ||
|
|
||
| expect(h.inviteVolunteer).not.toHaveBeenCalled() | ||
| expect(result.failedCount).toBe(1) | ||
| }) | ||
|
|
||
| it("creates an event and records it for undo", async () => { | ||
| h.loadScopeByRun.mockResolvedValue(scopeWith(null, "events")) | ||
|
|
||
| const result = await applyFn({ | ||
| data: { | ||
| importRunId: "aimp_1", | ||
| volunteerProposals: [], | ||
| eventProposals: [eventProposal()], | ||
| }, | ||
| }) | ||
|
|
||
| expect(h.createEvent).toHaveBeenCalledTimes(1) | ||
| expect(result.appliedCount).toBe(1) | ||
| expect(recordedEntities()).toEqual([ | ||
| { kind: "event_create", entityId: "trwk_new", rowKey: "er1" }, | ||
| ]) | ||
| }) | ||
| }) | ||
|
|
||
| describe("undoImportFn", () => { | ||
| it("deletes a still-pending created invitation", async () => { | ||
| h.loadScopeByRun.mockResolvedValue( | ||
| scopeWith( | ||
| JSON.stringify([ | ||
| { kind: "volunteer_invite", entityId: "tinv_new", rowKey: "r1" }, | ||
| ]), | ||
| ), | ||
| ) | ||
| mockDb.setMockSingleValue({ id: "tinv_new", acceptedAt: null }) | ||
|
|
||
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | ||
|
|
||
| expect(mockDb.delete).toHaveBeenCalledTimes(1) | ||
| expect(result.undoneCount).toBe(1) | ||
| expect(result.skippedCount).toBe(0) | ||
| }) | ||
|
|
||
| it("leaves an already-accepted invitation alone", async () => { | ||
| h.loadScopeByRun.mockResolvedValue( | ||
| scopeWith( | ||
| JSON.stringify([ | ||
| { kind: "volunteer_invite", entityId: "tinv_acc", rowKey: "r1" }, | ||
| ]), | ||
| ), | ||
| ) | ||
| mockDb.setMockSingleValue({ id: "tinv_acc", acceptedAt: new Date() }) | ||
|
|
||
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | ||
|
|
||
| expect(mockDb.delete).not.toHaveBeenCalled() | ||
| expect(result.undoneCount).toBe(0) | ||
| expect(result.skippedCount).toBe(1) | ||
| }) | ||
|
|
||
| it("removes a created event", async () => { | ||
| h.loadScopeByRun.mockResolvedValue( | ||
| scopeWith( | ||
| JSON.stringify([ | ||
| { kind: "event_create", entityId: "trwk_new", rowKey: "er1" }, | ||
| ]), | ||
| ), | ||
| ) | ||
|
|
||
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | ||
|
|
||
| expect(h.removeEvent).toHaveBeenCalledWith({ | ||
| data: { trackWorkoutId: "trwk_new", teamId: "team_org" }, | ||
| }) | ||
| expect(result.undoneCount).toBe(1) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
Add exactly one @lat reference next to each test case.
All it(...) blocks in this file are missing the required nearby spec reference comment. Add one // @lat: [[section-id]] next to each test (and avoid duplicates).
Suggested patch
describe("applyOrganizerImportFn", () => {
+ // `@lat`: [[organizer-import-apply-invite]]
it("invites a new volunteer and records the created invitation", async () => {
@@
+ // `@lat`: [[organizer-import-apply-idempotent]]
it("skips a row already written by a prior apply (idempotent)", async () => {
@@
+ // `@lat`: [[organizer-import-apply-no-email-fail]]
it("fails a create with no email and never invites", async () => {
@@
+ // `@lat`: [[organizer-import-apply-event-create]]
it("creates an event and records it for undo", async () => {
@@
describe("undoImportFn", () => {
+ // `@lat`: [[organizer-import-undo-pending-invite]]
it("deletes a still-pending created invitation", async () => {
@@
+ // `@lat`: [[organizer-import-undo-accepted-invite-skip]]
it("leaves an already-accepted invitation alone", async () => {
@@
+ // `@lat`: [[organizer-import-undo-event-remove]]
it("removes a created event", async () => {As per coding guidelines, tests must include exactly one nearby @lat: reference per test and not at file top.
📝 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
| it("invites a new volunteer and records the created invitation", async () => { | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(null)) | |
| mockDb.setMockSingleValue({ id: "tinv_new" }) // id-capture findFirst | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [volunteerProposal()], | |
| eventProposals: [], | |
| }, | |
| }) | |
| expect(h.inviteVolunteer).toHaveBeenCalledTimes(1) | |
| expect(result.appliedCount).toBe(1) | |
| expect(result.results[0]).toMatchObject({ | |
| rowKey: "r1", | |
| status: "applied", | |
| entityId: "tinv_new", | |
| }) | |
| expect(recordedEntities()).toEqual([ | |
| { kind: "volunteer_invite", entityId: "tinv_new", rowKey: "r1" }, | |
| ]) | |
| }) | |
| it("skips a row already written by a prior apply (idempotent)", async () => { | |
| const prior = JSON.stringify([ | |
| { kind: "volunteer_invite", entityId: "tinv_old", rowKey: "r1" }, | |
| ]) | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(prior)) | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [volunteerProposal({ rowKey: "r1" })], | |
| eventProposals: [], | |
| }, | |
| }) | |
| expect(h.inviteVolunteer).not.toHaveBeenCalled() | |
| expect(result.skippedCount).toBe(1) | |
| expect(result.results[0]).toMatchObject({ rowKey: "r1", status: "skipped" }) | |
| }) | |
| it("fails a create with no email and never invites", async () => { | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(null)) | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [volunteerProposal({ email: null })], | |
| eventProposals: [], | |
| }, | |
| }) | |
| expect(h.inviteVolunteer).not.toHaveBeenCalled() | |
| expect(result.failedCount).toBe(1) | |
| }) | |
| it("creates an event and records it for undo", async () => { | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(null, "events")) | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [], | |
| eventProposals: [eventProposal()], | |
| }, | |
| }) | |
| expect(h.createEvent).toHaveBeenCalledTimes(1) | |
| expect(result.appliedCount).toBe(1) | |
| expect(recordedEntities()).toEqual([ | |
| { kind: "event_create", entityId: "trwk_new", rowKey: "er1" }, | |
| ]) | |
| }) | |
| }) | |
| describe("undoImportFn", () => { | |
| it("deletes a still-pending created invitation", async () => { | |
| h.loadScopeByRun.mockResolvedValue( | |
| scopeWith( | |
| JSON.stringify([ | |
| { kind: "volunteer_invite", entityId: "tinv_new", rowKey: "r1" }, | |
| ]), | |
| ), | |
| ) | |
| mockDb.setMockSingleValue({ id: "tinv_new", acceptedAt: null }) | |
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | |
| expect(mockDb.delete).toHaveBeenCalledTimes(1) | |
| expect(result.undoneCount).toBe(1) | |
| expect(result.skippedCount).toBe(0) | |
| }) | |
| it("leaves an already-accepted invitation alone", async () => { | |
| h.loadScopeByRun.mockResolvedValue( | |
| scopeWith( | |
| JSON.stringify([ | |
| { kind: "volunteer_invite", entityId: "tinv_acc", rowKey: "r1" }, | |
| ]), | |
| ), | |
| ) | |
| mockDb.setMockSingleValue({ id: "tinv_acc", acceptedAt: new Date() }) | |
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | |
| expect(mockDb.delete).not.toHaveBeenCalled() | |
| expect(result.undoneCount).toBe(0) | |
| expect(result.skippedCount).toBe(1) | |
| }) | |
| it("removes a created event", async () => { | |
| h.loadScopeByRun.mockResolvedValue( | |
| scopeWith( | |
| JSON.stringify([ | |
| { kind: "event_create", entityId: "trwk_new", rowKey: "er1" }, | |
| ]), | |
| ), | |
| ) | |
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | |
| expect(h.removeEvent).toHaveBeenCalledWith({ | |
| data: { trackWorkoutId: "trwk_new", teamId: "team_org" }, | |
| }) | |
| expect(result.undoneCount).toBe(1) | |
| }) | |
| }) | |
| // `@lat`: [[organizer-import-apply-invite]] | |
| it("invites a new volunteer and records the created invitation", async () => { | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(null)) | |
| mockDb.setMockSingleValue({ id: "tinv_new" }) // id-capture findFirst | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [volunteerProposal()], | |
| eventProposals: [], | |
| }, | |
| }) | |
| expect(h.inviteVolunteer).toHaveBeenCalledTimes(1) | |
| expect(result.appliedCount).toBe(1) | |
| expect(result.results[0]).toMatchObject({ | |
| rowKey: "r1", | |
| status: "applied", | |
| entityId: "tinv_new", | |
| }) | |
| expect(recordedEntities()).toEqual([ | |
| { kind: "volunteer_invite", entityId: "tinv_new", rowKey: "r1" }, | |
| ]) | |
| }) | |
| // `@lat`: [[organizer-import-apply-idempotent]] | |
| it("skips a row already written by a prior apply (idempotent)", async () => { | |
| const prior = JSON.stringify([ | |
| { kind: "volunteer_invite", entityId: "tinv_old", rowKey: "r1" }, | |
| ]) | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(prior)) | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [volunteerProposal({ rowKey: "r1" })], | |
| eventProposals: [], | |
| }, | |
| }) | |
| expect(h.inviteVolunteer).not.toHaveBeenCalled() | |
| expect(result.skippedCount).toBe(1) | |
| expect(result.results[0]).toMatchObject({ rowKey: "r1", status: "skipped" }) | |
| }) | |
| // `@lat`: [[organizer-import-apply-no-email-fail]] | |
| it("fails a create with no email and never invites", async () => { | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(null)) | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [volunteerProposal({ email: null })], | |
| eventProposals: [], | |
| }, | |
| }) | |
| expect(h.inviteVolunteer).not.toHaveBeenCalled() | |
| expect(result.failedCount).toBe(1) | |
| }) | |
| // `@lat`: [[organizer-import-apply-event-create]] | |
| it("creates an event and records it for undo", async () => { | |
| h.loadScopeByRun.mockResolvedValue(scopeWith(null, "events")) | |
| const result = await applyFn({ | |
| data: { | |
| importRunId: "aimp_1", | |
| volunteerProposals: [], | |
| eventProposals: [eventProposal()], | |
| }, | |
| }) | |
| expect(h.createEvent).toHaveBeenCalledTimes(1) | |
| expect(result.appliedCount).toBe(1) | |
| expect(recordedEntities()).toEqual([ | |
| { kind: "event_create", entityId: "trwk_new", rowKey: "er1" }, | |
| ]) | |
| }) | |
| }) | |
| describe("undoImportFn", () => { | |
| // `@lat`: [[organizer-import-undo-pending-invite]] | |
| it("deletes a still-pending created invitation", async () => { | |
| h.loadScopeByRun.mockResolvedValue( | |
| scopeWith( | |
| JSON.stringify([ | |
| { kind: "volunteer_invite", entityId: "tinv_new", rowKey: "r1" }, | |
| ]), | |
| ), | |
| ) | |
| mockDb.setMockSingleValue({ id: "tinv_new", acceptedAt: null }) | |
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | |
| expect(mockDb.delete).toHaveBeenCalledTimes(1) | |
| expect(result.undoneCount).toBe(1) | |
| expect(result.skippedCount).toBe(0) | |
| }) | |
| // `@lat`: [[organizer-import-undo-accepted-invite-skip]] | |
| it("leaves an already-accepted invitation alone", async () => { | |
| h.loadScopeByRun.mockResolvedValue( | |
| scopeWith( | |
| JSON.stringify([ | |
| { kind: "volunteer_invite", entityId: "tinv_acc", rowKey: "r1" }, | |
| ]), | |
| ), | |
| ) | |
| mockDb.setMockSingleValue({ id: "tinv_acc", acceptedAt: new Date() }) | |
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | |
| expect(mockDb.delete).not.toHaveBeenCalled() | |
| expect(result.undoneCount).toBe(0) | |
| expect(result.skippedCount).toBe(1) | |
| }) | |
| // `@lat`: [[organizer-import-undo-event-remove]] | |
| it("removes a created event", async () => { | |
| h.loadScopeByRun.mockResolvedValue( | |
| scopeWith( | |
| JSON.stringify([ | |
| { kind: "event_create", entityId: "trwk_new", rowKey: "er1" }, | |
| ]), | |
| ), | |
| ) | |
| const result = await undoFn({ data: { importRunId: "aimp_1" } }) | |
| expect(h.removeEvent).toHaveBeenCalledWith({ | |
| data: { trackWorkoutId: "trwk_new", teamId: "team_org" }, | |
| }) | |
| expect(result.undoneCount).toBe(1) | |
| }) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts` around
lines 159 - 287, Each test block in the file is missing the required `@lat`
reference comment. Add a single line comment in the format // `@lat`:
[[section-id]] immediately before or next to each it(...) test case, including
the tests in the applyFn describe block (such as "invites a new volunteer and
records the created invitation", "skips a row already written by a prior apply
(idempotent)", "fails a create with no email and never invites", and "creates an
event and records it for undo") and the tests in the undoImportFn describe block
(such as "deletes a still-pending created invitation", "leaves an
already-accepted invitation alone", and "removes a created event"). Use
appropriate section identifiers for each test and ensure no duplicate `@lat`
references are used across tests.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts">
<violation number="1" location="apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts:159">
P3: Each `it(...)` block in this file is missing the required nearby `// @lat: [[section-id]]` spec reference comment. Per project coding guidelines, tests must include exactly one `@lat:` reference per test case (not at file top). Add a reference comment above each test linking it to the relevant spec section.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,287 @@ | |||
| import { FakeDrizzleDb } from "@repo/test-utils" | |||
Contributor
There was a problem hiding this comment.
P3: Each it(...) block in this file is missing the required nearby // @lat: [[section-id]] spec reference comment. Per project coding guidelines, tests must include exactly one @lat: reference per test case (not at file top). Add a reference comment above each test linking it to the relevant spec section.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts, line 159:
<comment>Each `it(...)` block in this file is missing the required nearby `// @lat: [[section-id]]` spec reference comment. Per project coding guidelines, tests must include exactly one `@lat:` reference per test case (not at file top). Add a reference comment above each test linking it to the relevant spec section.</comment>
<file context>
@@ -0,0 +1,287 @@
+})
+
+describe("applyOrganizerImportFn", () => {
+ it("invites a new volunteer and records the created invitation", async () => {
+ h.loadScopeByRun.mockResolvedValue(scopeWith(null))
+ mockDb.setMockSingleValue({ id: "tinv_new" }) // id-capture findFirst
</file context>
Completes the events story + the wireframe's inline-diff (pattern C): - planEventApply now emits "update" decisions, capturing a before-snapshot from the existing event (loadExistingEvents returns workoutId + current fields). Updates apply via saveCompetitionEventFn; undo restores the before-snapshot. - usePageIntent enables the event detail page (routeKind=event_detail). - review drawer renders update proposals as an inline field-level diff (before → after) and includes updates in confirm. - agent prompt: on event_detail, propose a single update with changedFields. - +10 tests (45 total); lat.md updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Contributor
There was a problem hiding this comment.
6 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts">
<violation number="1" location="apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts:326">
P2: Undo for event_update silently skips when before snapshot is missing required fields; no log emitted, masking potential data corruption.</violation>
</file>
<file name="apps/wodsmith-start/src/lib/organizer-file-import/validate.ts">
<violation number="1" location="apps/wodsmith-start/src/lib/organizer-file-import/validate.ts:256">
P2: Stale JSDoc comment claims updates are skipped, but the PR fully implements `update` outcomes in `EventApplyDecision` and `planEventApply`. The comment is also misplaced above `EventBeforeSnapshot` after the diff.</violation>
<violation number="2" location="apps/wodsmith-start/src/lib/organizer-file-import/validate.ts:321">
P2: `validateEventProposal` does not require a `scheme` for `create` actions, but `planEventApply` hard-fails create proposals without one. This inconsistency means a proposal can pass validation but fail at apply time, breaking the contract that these validators gate writes in both agent tools and the apply server function.</violation>
<violation number="3" location="apps/wodsmith-start/src/lib/organizer-file-import/validate.ts:352">
P1: Inconsistent scheme validation between `validateEventProposal` and `planEventApply` for update actions. `validateEventProposal` only checks `proposal.scheme` when present, while `planEventApply` merges `proposal.scheme ?? existing.scheme` and always validates the result. This causes proposals that only update non-scheme fields on events with legacy schemes to pass validation but fail at apply time.</violation>
</file>
<file name="apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts">
<violation number="1" location="apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts:265">
P2: Test for event update does not verify arguments passed to saveEvent, missing potential regressions in update payload</violation>
<violation number="2" location="apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts:273">
P2: Apply test's before-snapshot assertion omits scheme, scoreType, and description, masking potential gaps in snapshot capture</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| reason: "Update target event does not belong to this competition", | ||
| } | ||
| } | ||
| // Merge proposed changes over current values; only `scheme` is required |
Contributor
There was a problem hiding this comment.
P1: Inconsistent scheme validation between validateEventProposal and planEventApply for update actions. validateEventProposal only checks proposal.scheme when present, while planEventApply merges proposal.scheme ?? existing.scheme and always validates the result. This causes proposals that only update non-scheme fields on events with legacy schemes to pass validation but fail at apply time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/lib/organizer-file-import/validate.ts, line 352:
<comment>Inconsistent scheme validation between `validateEventProposal` and `planEventApply` for update actions. `validateEventProposal` only checks `proposal.scheme` when present, while `planEventApply` merges `proposal.scheme ?? existing.scheme` and always validates the result. This causes proposals that only update non-scheme fields on events with legacy schemes to pass validation but fail at apply time.</comment>
<file context>
@@ -280,43 +304,81 @@ export function planEventApply(
+ reason: "Update target event does not belong to this competition",
+ }
+ }
+ // Merge proposed changes over current values; only `scheme` is required
+ // by the underlying save, so fall back to the event's current scheme.
+ const scheme = proposal.scheme ?? existing.scheme
</file context>
| }) | ||
| skippedCount++ | ||
| } | ||
| } else { |
Contributor
There was a problem hiding this comment.
P2: Undo for event_update silently skips when before snapshot is missing required fields; no log emitted, masking potential data corruption.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/organizer-file-import-fns.ts, line 326:
<comment>Undo for event_update silently skips when before snapshot is missing required fields; no log emitted, masking potential data corruption.</comment>
<file context>
@@ -290,6 +296,38 @@ export const undoImportFn = createServerFn({ method: "POST" })
+ })
+ skippedCount++
+ }
+ } else {
+ skippedCount++
+ }
</file context>
| reason: validation.errors.join("; "), | ||
| } | ||
| } | ||
| if ( |
Contributor
There was a problem hiding this comment.
P2: validateEventProposal does not require a scheme for create actions, but planEventApply hard-fails create proposals without one. This inconsistency means a proposal can pass validation but fail at apply time, breaking the contract that these validators gate writes in both agent tools and the apply server function.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/lib/organizer-file-import/validate.ts, line 321:
<comment>`validateEventProposal` does not require a `scheme` for `create` actions, but `planEventApply` hard-fails create proposals without one. This inconsistency means a proposal can pass validation but fail at apply time, breaking the contract that these validators gate writes in both agent tools and the apply server function.</comment>
<file context>
@@ -280,43 +304,81 @@ export function planEventApply(
+ reason: validation.errors.join("; "),
+ }
+ }
+ if (
+ !proposal.scheme ||
+ !options.allowedSchemes.includes(proposal.scheme)
</file context>
| * Per-row decision for applying event proposals. Pure. MVP supports `create` | ||
| * only; updates are surfaced as skipped until the inline-diff write path lands. | ||
| */ | ||
| export interface EventBeforeSnapshot { |
Contributor
There was a problem hiding this comment.
P2: Stale JSDoc comment claims updates are skipped, but the PR fully implements update outcomes in EventApplyDecision and planEventApply. The comment is also misplaced above EventBeforeSnapshot after the diff.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/lib/organizer-file-import/validate.ts, line 256:
<comment>Stale JSDoc comment claims updates are skipped, but the PR fully implements `update` outcomes in `EventApplyDecision` and `planEventApply`. The comment is also misplaced above `EventBeforeSnapshot` after the diff.</comment>
<file context>
@@ -249,6 +253,14 @@ export function validateEventProposal(
* Per-row decision for applying event proposals. Pure. MVP supports `create`
* only; updates are surfaced as skipped until the inline-diff write path lands.
*/
+export interface EventBeforeSnapshot {
+ workoutId: string
+ name: string
</file context>
| before?: { name?: string; workoutId?: string } | ||
| } | ||
| expect(entity).toMatchObject({ kind: "event_update", entityId: "trwk_1" }) | ||
| expect(entity.before).toMatchObject({ name: "Old name", workoutId: "wkt_1" }) |
Contributor
There was a problem hiding this comment.
P2: Apply test's before-snapshot assertion omits scheme, scoreType, and description, masking potential gaps in snapshot capture
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts, line 273:
<comment>Apply test's before-snapshot assertion omits scheme, scoreType, and description, masking potential gaps in snapshot capture</comment>
<file context>
@@ -231,6 +233,45 @@ describe("applyOrganizerImportFn", () => {
+ before?: { name?: string; workoutId?: string }
+ }
+ expect(entity).toMatchObject({ kind: "event_update", entityId: "trwk_1" })
+ expect(entity.before).toMatchObject({ name: "Old name", workoutId: "wkt_1" })
+ })
})
</file context>
Suggested change
| expect(entity.before).toMatchObject({ name: "Old name", workoutId: "wkt_1" }) | |
| expect(entity.before).toMatchObject({ name: "Old name", workoutId: "wkt_1", scheme: "time", scoreType: null, description: "old" }) |
| }, | ||
| }) | ||
|
|
||
| expect(h.saveEvent).toHaveBeenCalledTimes(1) |
Contributor
There was a problem hiding this comment.
P2: Test for event update does not verify arguments passed to saveEvent, missing potential regressions in update payload
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/test/server-fns/organizer-file-import-fns.test.ts, line 265:
<comment>Test for event update does not verify arguments passed to saveEvent, missing potential regressions in update payload</comment>
<file context>
@@ -231,6 +233,45 @@ describe("applyOrganizerImportFn", () => {
+ },
+ })
+
+ expect(h.saveEvent).toHaveBeenCalledTimes(1)
+ expect(result.appliedCount).toBe(1)
+ const entity = recordedEntities()[0] as {
</file context>
Wires the previously-unused checksum infrastructure (the (competitionId, checksum) index): findPriorAppliedImport looks up a prior APPLIED run of the same file, and the agent surfaces a soft "already imported" warning so an organizer doesn't accidentally double-apply a re-dropped file. +3 unit tests (48 total). No new dependencies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
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
2 participants
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.
Replaces a stale, unrelated plan.md (soft-delete registrations) with a step-by-step guide for the drag-and-drop organizer file import agent: data model, private R2 upload, Cloudflare Agent (proposal-only), the confirm/apply + undo path, and the drawer/dock/inline-diff UI from the wireframe. Grounded in the existing AI judge-scheduler patterns.
Claude-Session: https://claude.ai/code/session_01PnNx8rXbhsgA3oCYj2RFRt
Summary by cubic
Implements the organizer file-drop import agent end to end: private upload, reviewable proposals, confirm/apply with undo, event creates from Events and single-event updates with inline diff. Adds checksum-based duplicate-import detection that warns on previously applied files.
New Features
agent_import_runswith status, checksum (duplicate-import detection), and applied-entity receipt.AI_FILE_IMPORT; access checks mirror judge-scheduler.OrganizerFileImportAgentDO that parses uploaded files, loads context, drafts proposals, supports refine/clarify; proposal-only (no DB writes)./api/agent-import/uploadto R2 with SHA-256;createImportRunFnandloadFileImportContextFn.papaparse; Zod schemas and pure validators; plannersplanVolunteerApplyandplanEventApply(emits updates) with unit tests; orchestration tests for apply/undo server-fns.ImportShellwraps organizer layout (dock + drag overlay on Volunteers/Judges/Events list and event detail);ImportReviewDrawerstreams proposals, refine prompt, per-row exclude, confirm (mode-aware), undo with receipt, and inline field-level diff for event updates.server.ts/alchemy.run.ts; route registered inrouteTree.gen.ts; env addsORGANIZER_FILE_IMPORT_AGENT.Migration
agent_import_runs.AI_FILE_IMPORTentitlement to expose the dock and upload flow.Written for commit 4a28045. Summary will update on new commits.
Summary by CodeRabbit