Skip to content

Conversation

KATO-Hiro
Copy link
Collaborator

@KATO-Hiro KATO-Hiro commented Sep 28, 2025

close #2627

Summary by CodeRabbit

  • New Features
    • Backend groundwork enabling the same problem to be associated with multiple contests (feature gated; no immediate user-facing UI changes).
  • Documentation
    • Added comprehensive plans, phased migration strategy, import/admin UI scaffold, risks, testing, and rollout notes for multi-contest problem mapping.
  • Chores
    • Database schema and indexes added to support contest–task pairing; removed an obsolete preview configuration flag.
  • Tests
    • Added unit tests for the new mapping adapter/key utilities.

Copy link

coderabbitai bot commented Sep 28, 2025

Walkthrough

Adds planning docs for multi-contest task mapping, introduces a new Prisma model ContestTaskPair, SQL migration to create public.contesttaskpair with indexes and a composite unique constraint, updates ERD, and adds TypeScript aliases. No runtime service logic is changed in this diff.

Changes

Cohort / File(s) Summary
Planning docs
docs/dev-notes/2025-09-17/contest-task-mapping/initial_plan.md, docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md
New design and migration plans detailing support for multiple contests per problem, the ContestTaskPair model, phased migration, compatibility layer, feature-flag strategy, CRUD outlines, import/UI scaffolding, testing and observability notes.
Prisma schema & ERD
prisma/schema.prisma, prisma/ERD.md
Adds ContestTaskPair model to Prisma schema and documents contesttaskpair entity in ERD; ERD also updates/remove several explicit relation labels.
SQL migration
prisma/migrations/20250928072152_create_contest_task_pair/migration.sql
Creates table public.contesttaskpair (id PK, contestId, taskTableIndex, taskId, createdAt, updatedAt), adds index on contestId, and unique index on (contestId, taskId).
TypeScript types
src/lib/types/contest_task_pair.ts
Adds type aliases ContestTaskPair and ContestTaskPairs mapped to Prisma client types.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Admin as Admin / Seeder
  participant App as App Service (future CRUD)
  participant Prisma as Prisma Client
  participant DB as Postgres

  rect rgb(238,245,255)
    note right of App: Create / upsert ContestTaskPair (planned flow)
    Admin->>App: createContestTaskPair(contestId, taskId, taskTableIndex)
    App->>Prisma: upsert ContestTaskPair
    Prisma->>DB: INSERT ... ON CONFLICT (contestId, taskId) DO UPDATE
    DB-->>Prisma: row
    Prisma-->>App: ContestTaskPair
    App-->>Admin: result
  end

  rect rgb(245,238,255)
    note right of App: Read by contest (planned flow)
    Admin->>App: getContestTaskPairs(contestId)
    App->>Prisma: findMany where contestId
    Prisma->>DB: SELECT ... WHERE contestId=?
    DB-->>Prisma: rows
    Prisma-->>App: ContestTaskPairs
    App-->>Admin: list
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I plant a pair where contests meet, hop, hop—so spry,
A tiny table, keyed and neat beneath the sky.
Migrations hum, types whisper names,
Indices guard the rabbit lanes.
With a nibble and a thump, the mapping's set—hooray! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title "✨ Create ContestTaskPair table (#2627)" clearly reflects the primary change of adding the ContestTaskPair table but includes an emoji and the linked issue number which are unnecessary noise per guidelines. Please remove the leading emoji and the redundant issue number to simplify the title to something like "Create ContestTaskPair table" for clarity and conciseness.
Out of Scope Changes Check ⚠️ Warning The pull request includes extensive documentation files under docs/dev-notes outlining a phased implementation plan and utilities for ContestTaskPair mapping, which exceed the linked issue’s scope of simply creating the table and defining its attributes. Please remove or move the planning and scaffolding documentation in docs/dev-notes to a separate RFC or feature-design issue, keeping this PR focused strictly on adding the ContestTaskPair table and its schema definitions.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check ✅ Passed The pull request fully implements the objective from issue #2627 by introducing the ContestTaskPair table in the database schema with the required fields and attributes, adding the corresponding Prisma model, ERD documentation, migration script, and related type definitions to support the new entity.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch #2627

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between f7485e5 and 5daa1bf.

📒 Files selected for processing (1)
  • docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (1 hunks)
🔇 Additional comments (4)
docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (4)

217-222: Fix the enhancedMap generic type

The constructor still types enhancedMap as Map<TaskResultByContestTaskPair, T>, i.e. using a whole Map type as the key. That won’t compile and was already raised earlier. Please change it to use the key alias directly (e.g. Map<ContestTaskPairKey, T>).

-    private enhancedMap?: Map<TaskResultByContestTaskPair, T>, // 新形式: contestId:taskId -> value
+    private enhancedMap?: Map<ContestTaskPairKey, T>, // 新形式: contestId:taskId -> value

302-357: Replace console.assert with throwing guards

console.assert only logs; it never throws. Yet the plan (and test snippet below) expects adapter.get('', …) and makeKey('', …) to throw. Please replace these assertions with explicit guards that throw Errors so invalid inputs can’t slip through.

-    console.assert(
-      typeof taskId === 'string' && taskId.length > 0,
-      'taskId must be non-empty string',
-    );
+    if (typeof taskId !== 'string' || taskId.length === 0) {
+      throw new Error('taskId must be a non-empty string');
+    }

(and similarly for contestId and the other assertion.)


481-488: Guard status lookups before dereferencing

statusById.get(answer?.status_id) can return undefined. Dereferencing it as status.status_name will immediately crash when the status map lacks that entry. Add a null check (or use optional chaining with sensible fallbacks) before copying fields to taskResult.

-      const status = statusById.get(answer?.status_id);
-      taskResult.status_name = status.status_name;
-      taskResult.submission_status_image_path = status.image_path;
-      taskResult.submission_status_label_name = status.label_name;
-      taskResult.is_ac = status.is_ac;
+      const status = statusById.get(answer?.status_id);
+      if (status) {
+        taskResult.status_name = status.status_name;
+        taskResult.submission_status_image_path = status.image_path;
+        taskResult.submission_status_label_name = status.label_name;
+        taskResult.is_ac = status.is_ac;
+      }

603-611: Break the infinite recursion in the monitoring wrapper

The profiling wrapper calls getTaskResults recursively, guaranteeing a stack overflow. Wrap the real implementation instead (e.g. call getTaskResultsCore or similar) so the monitoring code just delegates.

-  const results = await getTaskResults(userId);
+  const results = await getTaskResultsCore(userId);

(with the core logic moved into a separate function.)


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (5)
prisma/schema.prisma (2)

111-123: Add supporting index for common access pattern by (contestId, taskTableIndex).

Fetching tasks within a contest ordered by letter/index is common; add a composite index. Consider a uniqueness guarantee if your domain requires one.

 model ContestTaskPair {
   // ...
   @@unique([contestId, taskId])
   @@index([contestId])
+  @@index([contestId, taskTableIndex])
+  // If the contest letter must be unique within a contest, enforce it:
+  // @@unique([contestId, taskTableIndex])
   @@map("contesttaskpair")
 }

113-113: Consider using database UUID type for id (optional).

If you’re okay with a slight inconsistency vs. existing tables, prefer @db.Uuid + @default(uuid()) so Postgres stores UUIDs natively.

-  id             String   @id @default(uuid())
+  id             String   @id @db.Uuid @default(uuid())
prisma/migrations/20250928072152_create_contest_task_pair/migration.sql (1)

13-17: Add helpful indexes for query patterns.

Add indexes to support reverse lookups and in‑contest ordering.

 CREATE INDEX "contesttaskpair_contestId_idx" ON "public"."contesttaskpair"("contestId");
+CREATE INDEX "contesttaskpair_taskId_idx" ON "public"."contesttaskpair"("taskId");
+CREATE INDEX "contesttaskpair_contestId_taskTableIndex_idx" ON "public"."contesttaskpair"("contestId","taskTableIndex");
-- Optionally enforce uniqueness if required by domain:
-- CREATE UNIQUE INDEX "contesttaskpair_contestId_taskTableIndex_key" ON "public"."contesttaskpair"("contestId","taskTableIndex");
prisma/ERD.md (1)

129-137: Show relation to Task in ERD (after schema change).

Once you add ContestTaskPair.task -> Task.task_id, re-run ERD generation so the diagram reflects the link.

docs/dev-notes/2025-09-17/contest-task-mapping/initial_plan.md (1)

176-189: Fix markdownlint MD040: specify language on code fences.

Add a language to the directory tree block (e.g., text) to satisfy linting.

-```
+```text
 src/routes/admin/import/
 ├── problems/           # 問題データインポート
 │   ├── +page.svelte
 │   └── types.ts
 ├── mapping/            # コンテスト-問題マッピング
 │   ├── +page.svelte
 │   ├── bulk/
 │   └── selective/
 └── shared/            # 共通コンポーネント
     ├── ImportStatus.svelte
     └── ValidationResults.svelte

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used**: CodeRabbit UI

**Review profile**: CHILL

**Plan**: Pro

**Cache: Disabled due to data retention organization setting**

**Knowledge base: Disabled due to data retention organization setting**

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 07586d60698fefbae50a0e250b2eac2b10c73614 and 858761171777872228814448352945cf3c73c8d0.

</details>

<details>
<summary>📒 Files selected for processing (6)</summary>

* `docs/dev-notes/2025-09-17/contest-task-mapping/initial_plan.md` (1 hunks)
* `docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md` (1 hunks)
* `prisma/ERD.md` (2 hunks)
* `prisma/migrations/20250928072152_create_contest_task_pair/migration.sql` (1 hunks)
* `prisma/schema.prisma` (2 hunks)
* `src/lib/types/contest_task_pair.ts` (1 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>

<details>
<summary>docs/dev-notes/2025-09-17/contest-task-mapping/initial_plan.md</summary>

176-176: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary>

* GitHub Check: preview

</details>

<details>
<summary>🔇 Additional comments (4)</summary><blockquote>

<details>
<summary>prisma/schema.prisma (2)</summary><blockquote>

`8-9`: **Confirm binaryTargets match deploy runtime (OpenSSL 3.x).**

If your deploy images aren’t RHEL/OpenSSL 3.x, Prisma engines may fail to load. Verify the runtime and adjust if needed.

---

`35-39`: **Nice: camelCase for new/modified models.**

This follows the note about Lucia v3 and gradual migration. No action needed.

</blockquote></details>
<details>
<summary>src/lib/types/contest_task_pair.ts (1)</summary><blockquote>

`1-5`: **Types look good and align with Prisma model.**

Lightweight aliases; ready for use in services.


If you maintain barrel exports (e.g., `src/lib/types/index.ts`), add these types there for discoverability.

</blockquote></details>
<details>
<summary>docs/dev-notes/2025-09-17/contest-task-mapping/initial_plan.md (1)</summary><blockquote>

`121-141`: **Good phased plan and adapter sketch.**

The compatibility layer and key format are pragmatic for staged migration.

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8587611 and 26676ff.

📒 Files selected for processing (1)
  • docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md

[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.

🔇 Additional comments (1)
docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (1)

302-356: Replace assertions with throwing guards in TaskMapAdapter.

Line 304 still uses console.assert, which only logs even when the condition fails. The test plan in this document expects adapter.get('') and adapter.get('task1', '') to throw, so copying this code will keep the bug we discussed earlier. Please swap the assertions for explicit guards that throw Errors when the inputs are invalid.

@@
-    console.assert(
-      typeof taskId === 'string' && taskId.length > 0,
-      'taskId must be non-empty string',
-    );
+    if (typeof taskId !== 'string' || taskId.length === 0) {
+      throw new Error('taskId must be a non-empty string');
+    }
@@
-    console.assert(
-      typeof contestId === 'string' && contestId.length > 0,
-      'contestId must be non-empty string',
-    );
-    console.assert(
-      typeof taskId === 'string' && taskId.length > 0,
-      'taskId must be non-empty string',
-    );
+    if (typeof contestId !== 'string' || contestId.length === 0) {
+      throw new Error('contestId must be a non-empty string');
+    }
+    if (typeof taskId !== 'string' || taskId.length === 0) {
+      throw new Error('taskId must be a non-empty string');
+    }

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (5)
docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (5)

308-318: Avoid performance.now portability issues in Node

performance may not be in lib.dom for TS; prefer Date.now() or import from node:perf_hooks.

Apply:

-    const start = performance.now();
+    const start = Date.now();
...
-            `TaskMapAdapter.get: enhanced hit (${(performance.now() - start).toFixed(2)}ms)`,
+            `TaskMapAdapter.get: enhanced hit (${(Date.now() - start).toFixed(2)}ms)`,
...
-        console.log(`TaskMapAdapter.get: legacy hit (${(performance.now() - start).toFixed(2)}ms)`);
+        console.log(`TaskMapAdapter.get: legacy hit (${(Date.now() - start).toFixed(2)}ms)`);
...
-              `TaskMapAdapter.get: enhanced scan hit (${(performance.now() - start).toFixed(2)}ms)`,
+              `TaskMapAdapter.get: enhanced scan hit (${(Date.now() - start).toFixed(2)}ms)`,

Or import { performance } from 'node:perf_hooks' and keep performance.now.

Also applies to: 326-327, 337-338


89-117: Race between existence check and create

Between findUnique and create, concurrent writers can still hit P2002. Prefer upsert (if updating taskTableIndex is acceptable) or catch P2002 explicitly.

Apply (catch P2002, preserving “create-only” semantics):

-    // 新規レコード作成
-    const contestTaskPair = await db.contestTaskPair.create({
-      data: {
-        contestId,
-        taskTableIndex,
-        taskId,
-      },
-    });
+    // 新規レコード作成(競合時は穏当に無視)
+    let contestTaskPair: ContestTaskPair | undefined;
+    try {
+      contestTaskPair = await db.contestTaskPair.create({
+        data: { contestId, taskTableIndex, taskId },
+      });
+    } catch (e: unknown) {
+      if (e && typeof e === 'object' && 'code' in e && (e as any).code === 'P2002') {
+        console.log(`ContestTaskPair already exists (race): contestId=${contestId}, taskId=${taskId}`);
+        return;
+      }
+      throw e;
+    }

Alternatively, use upsert to update taskTableIndex when the pair exists.


202-204: Unify type aliases for contest-task keyed maps

Names and shapes diverge: TaskResultMapByContestTaskPair vs TaskResultByContestTaskPair. Standardize on one generic alias and use consistently in snippets and imports.

Apply:

-// マップの型(明示的)
-type TaskResultMapByContestTaskPair = Map<ContestTaskPairKey, TaskResult>;
+// Generic map type keyed by ContestTaskPairKey
+export type MapByContestTaskPair<T> = Map<ContestTaskPairKey, T>;
-type TaskResultByContestTaskPair<T> = Map<ContestTaskPairKey, T>;
+export type TaskResultByContestTaskPair<T> = Map<ContestTaskPairKey, T>;

And ensure service usage/imports reference the exported alias:

-import { TaskMapAdapter } from '$lib/utils/task_results';
+import { TaskMapAdapter, TaskResultByContestTaskPair } from '$lib/utils/task_results';

Also applies to: 283-286, 440-446


409-418: Remove unused contestTaskPairMap or return/use it

contestTaskPairMap is built but unused. Drop it or include it in the returned object and use downstream.

Apply:

-  // contestId:taskId -> Task のマップ(TaskResult 作成で直接参照しやすい)
-  const contestTaskPairMap = new Map<string, Task>();
-  for (const t of tasks) {
-    const contests = contestTaskPairs.get(t.task_id) ?? [t.contest_id];
-    for (const c of contests) {
-      contestTaskPairMap.set(`${c}:${t.task_id}`, t);
-    }
-  }
-
-  return { tasks, contestTaskPairs, contestTaskPairMap };
+  return { tasks, contestTaskPairs };

145-149: Update only mutable fields in updateContestTaskPair

Passing contestId/taskId in data is redundant and risks accidental key changes. Update taskTableIndex only.

Apply:

   data: {
-    contestId,
-    taskTableIndex,
-    taskId,
+    taskTableIndex,
   },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 26676ff and f7485e5.

📒 Files selected for processing (1)
  • docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: preview
🔇 Additional comments (3)
docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md (3)

425-427: Ensure types are exported/imported consistently

TaskResultByContestTaskPair is used here but not imported in the snippet. Export it from utils and import alongside TaskMapAdapter.

Apply:

-import { TaskMapAdapter } from '$lib/utils/task_results';
+import { TaskMapAdapter, TaskResultByContestTaskPair } from '$lib/utils/task_results';

Please confirm the actual file exports/imports match this.

Also applies to: 440-446


373-381: Syntax error: missing closing brace in createContestTaskPairKey

Close the second if block before returning; otherwise this snippet won’t compile.

Apply:

 export function createContestTaskPairKey(contestId: string, taskId: string): ContestTaskPairKey {
   if (!contestId || contestId.trim() === '') {
     throw new Error('contestId must be a non-empty string');
   }
   if (!taskId || taskId.trim() === '') {
     throw new Error('taskId must be a non-empty string');
   }
+
   return `${contestId}:${taskId}`;
 }

302-307: Replace console.assert with throwing guards (aligns with tests)

console.assert won’t throw; tests expect errors on invalid inputs. Add explicit validation in get and makeKey (also validate contestId when provided).

Apply:

   get(taskId: string, contestId?: string): T | undefined {
-    console.assert(
-      typeof taskId === 'string' && taskId.length > 0,
-      'taskId must be non-empty string',
-    );
+    if (typeof taskId !== 'string' || taskId.trim().length === 0) {
+      throw new Error('taskId must be a non-empty string');
+    }
+    if (contestId !== undefined && (typeof contestId !== 'string' || contestId.trim().length === 0)) {
+      throw new Error('contestId must be a non-empty string when provided');
+    }
   private makeKey(contestId: string, taskId: string): ContestTaskPairKey {
-    console.assert(
-      typeof contestId === 'string' && contestId.length > 0,
-      'contestId must be non-empty string',
-    );
-    console.assert(
-      typeof taskId === 'string' && taskId.length > 0,
-      'taskId must be non-empty string',
-    );
+    if (typeof contestId !== 'string' || contestId.trim().length === 0) {
+      throw new Error('contestId must be a non-empty string');
+    }
+    if (typeof taskId !== 'string' || taskId.trim().length === 0) {
+      throw new Error('taskId must be a non-empty string');
+    }

Also applies to: 348-356, 241-246

Comment on lines +217 to +225
class TaskResultMapAdapter<T> {
constructor(
private legacyMap?: Map<string, T>, // 既存: taskId -> value
private enhancedMap?: Map<TaskResultByContestTaskPair, T>, // 新形式: contestId:taskId -> value
) {}

private makeKey(contestId: string, taskId: string): ContestTaskPairKey {
return `${contestId}:${taskId}`;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Type error: wrong Map key type for enhancedMap

The key should be ContestTaskPairKey, not TaskResultByContestTaskPair (which is a Map type). Current generics won’t type-check.

Apply:

 class TaskResultMapAdapter<T> {
   constructor(
     private legacyMap?: Map<string, T>, // 既存: taskId -> value
-    private enhancedMap?: Map<TaskResultByContestTaskPair, T>, // 新形式: contestId:taskId -> value
+    private enhancedMap?: Map<ContestTaskPairKey, T>, // 新形式: contestId:taskId -> value
   ) {}
📝 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
class TaskResultMapAdapter<T> {
constructor(
private legacyMap?: Map<string, T>, // 既存: taskId -> value
private enhancedMap?: Map<TaskResultByContestTaskPair, T>, // 新形式: contestId:taskId -> value
) {}
private makeKey(contestId: string, taskId: string): ContestTaskPairKey {
return `${contestId}:${taskId}`;
}
class TaskResultMapAdapter<T> {
constructor(
private legacyMap?: Map<string, T>, // 既存: taskId -> value
private enhancedMap?: Map<ContestTaskPairKey, T>, // 新形式: contestId:taskId -> value
) {}
private makeKey(contestId: string, taskId: string): ContestTaskPairKey {
return `${contestId}:${taskId}`;
}
}
🤖 Prompt for AI Agents
In docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md around lines 217
to 225, the enhancedMap field is typed as Map<TaskResultByContestTaskPair, T>
which is incorrect because TaskResultByContestTaskPair is a Map type; change the
enhancedMap declaration to use ContestTaskPairKey as the key type (i.e.
Map<ContestTaskPairKey, T>) and ensure the makeKey method’s return type is
ContestTaskPairKey so the key types align.

Comment on lines +361 to +362
// 他のメソッド (has, set, delete) も同様に実装
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Adapter method mismatch: has/set/delete not implemented but used

TaskMapAdapter is used with adapter.has(...); the utils snippet only implements get. This will not compile at usage sites.

Apply:

-  // 他のメソッド (has, set, delete) も同様に実装
+  has(taskId: string, contestId?: string): boolean {
+    if (contestId && this.enhancedMap) {
+      return this.enhancedMap.has(this.makeKey(contestId, taskId));
+    }
+    if (this.legacyMap?.has(taskId)) return true;
+    if (this.enhancedMap) {
+      for (const key of this.enhancedMap.keys()) {
+        if (key.endsWith(`:${taskId}`)) return true;
+      }
+    }
+    return false;
+  }
+
+  set(taskId: string, value: T, contestId?: string): void {
+    this.legacyMap?.set(taskId, value);
+    if (contestId && this.enhancedMap) {
+      this.enhancedMap.set(this.makeKey(contestId, taskId), value);
+    }
+  }
+
+  delete(taskId: string, contestId?: string): boolean {
+    let ok = false;
+    if (contestId && this.enhancedMap) {
+      ok = this.enhancedMap.delete(this.makeKey(contestId, taskId)) || ok;
+    }
+    if (this.legacyMap) {
+      ok = this.legacyMap.delete(taskId) || ok;
+    }
+    return ok;
+  }

Also applies to: 480-482

🤖 Prompt for AI Agents
In docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md around lines
361-362 (and likewise lines 480-482), the TaskMapAdapter utility only implements
get(...) but calling sites use adapter.has(...), adapter.set(...), and
adapter.delete(...); add matching method signatures and implementations for has,
set, and delete to the adapter interface/implementation so compilation succeeds.
Implement has to return a boolean presence check, set to store/update the value,
and delete to remove the key, ensuring their behavior mirrors get (including
async/await if get is async) and update any type/interface definitions and
usages accordingly.

Comment on lines +480 to +488
if (isLoggedIn && adapter.has(task.task_id, task.contest_id)) {
const answer = adapter.get(task.task_id, task.contest_id);
const status = statusById.get(answer?.status_id);
taskResult.status_name = status.status_name;
taskResult.submission_status_image_path = status.image_path;
taskResult.submission_status_label_name = status.label_name;
taskResult.is_ac = status.is_ac;
taskResult.updated_at = answer?.updated_at ?? taskResult.updated_at;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard against undefined status to avoid runtime crash

statusById.get(...) can be undefined; dereferencing will throw.

Apply:

-    if (isLoggedIn && adapter.has(task.task_id, task.contest_id)) {
-      const answer = adapter.get(task.task_id, task.contest_id);
-      const status = statusById.get(answer?.status_id);
-      taskResult.status_name = status.status_name;
-      taskResult.submission_status_image_path = status.image_path;
-      taskResult.submission_status_label_name = status.label_name;
-      taskResult.is_ac = status.is_ac;
-      taskResult.updated_at = answer?.updated_at ?? taskResult.updated_at;
-    }
+    if (isLoggedIn && adapter.has(task.task_id, task.contest_id)) {
+      const answer = adapter.get(task.task_id, task.contest_id);
+      const status = statusById.get(answer?.status_id);
+      if (status) {
+        taskResult.status_name = status.status_name;
+        taskResult.submission_status_image_path = status.image_path;
+        taskResult.submission_status_label_name = status.label_name;
+        taskResult.is_ac = status.is_ac;
+      }
+      taskResult.updated_at = answer?.updated_at ?? taskResult.updated_at;
+    }
📝 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
if (isLoggedIn && adapter.has(task.task_id, task.contest_id)) {
const answer = adapter.get(task.task_id, task.contest_id);
const status = statusById.get(answer?.status_id);
taskResult.status_name = status.status_name;
taskResult.submission_status_image_path = status.image_path;
taskResult.submission_status_label_name = status.label_name;
taskResult.is_ac = status.is_ac;
taskResult.updated_at = answer?.updated_at ?? taskResult.updated_at;
}
if (isLoggedIn && adapter.has(task.task_id, task.contest_id)) {
const answer = adapter.get(task.task_id, task.contest_id);
const status = statusById.get(answer?.status_id);
if (status) {
taskResult.status_name = status.status_name;
taskResult.submission_status_image_path = status.image_path;
taskResult.submission_status_label_name = status.label_name;
taskResult.is_ac = status.is_ac;
}
taskResult.updated_at = answer?.updated_at ?? taskResult.updated_at;
}
🤖 Prompt for AI Agents
In docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md around lines 480
to 488, statusById.get(answer?.status_id) can return undefined and the
subsequent property dereferences will crash; update the block to first check
that status is defined (e.g., if (status) { ... }) or use safe optional chaining
with sensible fallbacks so you only assign status_name, image_path, label_name,
is_ac, and updated_at when status exists, and preserve existing taskResult
values or set defaults when it does not.

Comment on lines +602 to +621
// console.log ベースの監視(開発環境のみ)
export async function getTaskResults(userId: string): Promise<TaskResults> {
if (process.env.NODE_ENV === 'development') {
console.time('getTaskResults');
console.log(`Processing ${answers.size} answers for user ${userId}`);
}

const results = await getTaskResults(userId);

if (process.env.NODE_ENV === 'development') {
console.log(`Generated ${results.length} task results`);
console.timeEnd('getTaskResults');

// メモリ使用量確認
const memory = process.memoryUsage();
console.log(`Memory usage: ${Math.round(memory.heapUsed / 1024 / 1024)}MB`);
}

return results;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Infinite recursion in monitoring snippet

getTaskResults calls itself; this will stack overflow. Wrap the real implementation instead.

Apply:

-export async function getTaskResults(userId: string): Promise<TaskResults> {
+export async function getTaskResults(userId: string): Promise<TaskResults> {
   if (process.env.NODE_ENV === 'development') {
     console.time('getTaskResults');
-    console.log(`Processing ${answers.size} answers for user ${userId}`);
+    console.log(`Processing task results for user ${userId}`);
   }
 
-  const results = await getTaskResults(userId);
+  const results = await getTaskResultsCore(userId);
 
   if (process.env.NODE_ENV === 'development') {
     console.log(`Generated ${results.length} task results`);
     console.timeEnd('getTaskResults');
@@
   return results;
 }
+
+// Extract the core implementation to avoid recursion in the wrapper.
+async function getTaskResultsCore(userId: string): Promise<TaskResults> {
+  // ...existing non-profiling logic...
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In docs/dev-notes/2025-09-23/contest-task-pair-mapping/plan.md around lines 602
to 621, the exported getTaskResults wraps monitoring but currently calls itself
(infinite recursion); extract or call the real implementation instead (e.g.,
rename the actual logic to fetchTaskResults or _getTaskResults and call that
inside the monitoring wrapper), ensure the wrapper logs/times only in
development, and return the result from the underlying function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DBのテーブルを作成・属性を定義
1 participant