diff --git a/.claude/rules/svelte-runes.md b/.claude/rules/svelte-runes.md index 86145381b..bfea707a0 100644 --- a/.claude/rules/svelte-runes.md +++ b/.claude/rules/svelte-runes.md @@ -33,6 +33,14 @@ Inside `$derived`, plain `new Map()` suffices — reactive dependency tracked at - Use `.by()` for multi-statement: `$derived.by(() => { ... })` - Inside `$effect`, use `$store` syntax, not `get(store)` (bypasses signal graph) +## `$derived` Tracks Reactive Reads Inside Called Functions + +`$derived(fn)` tracks every reactive read inside `fn`, **including reads inside helper functions** called by `fn`. If a helper reads a `$state` array, the derived re-runs whenever that array changes. + +**Accumulation trap**: if the helper transforms data (e.g. `{ ...item, title: prefix + item.title }`) and the result is later written back to the source array (optimistic update), the next re-run reads the already-transformed value and transforms it again. + +Fix: put display formatting only in the leaf view (e.g. `$derived displayTitle` in a cell component), never inside a derived that feeds mutable state. + ## Optimistic Updates Derive computed fields from canonical data source, not re-implement inline. Divergence → "works after reload" bugs. diff --git a/.claude/skills/add-contest-table-provider/instructions.md b/.claude/skills/add-contest-table-provider/instructions.md index 2c987bf52..7f87f3c5c 100644 --- a/.claude/skills/add-contest-table-provider/instructions.md +++ b/.claude/skills/add-contest-table-provider/instructions.md @@ -30,7 +30,7 @@ Step 0 (seed check) is already done. Confirm the following before touching code: - Year/ID range: oldest and latest? Export both as named constants so tests can reference them. - Iteration order: latest-first so newest table renders on top. - `task_table_index` values numeric strings? → override `getHeaderIdsForTask`; sort with `Number(a) - Number(b)`. -- Display-only title transform needed (e.g. prepend letter)? → override `generateTable` for the transform AND override `getHeaderIdsForTask` using the same key derivation; mismatched keys between the two methods cause missing cells. +- Display-only positional label needed (e.g. prepend "A. ")? → override `getTaskLabels` to return `{ [contestId]: { index: letter } }`; **never mutate title inside `generateTable`** (transformed objects written back via optimistic update cause prefix accumulation on the next `$derived` re-run). - Known edge cases where the default algorithm breaks? → add a `Record>` module-level override map keyed by contest_id; exercise the override path in tests by mutating the export in `beforeEach` and cleaning up in `afterEach`. **Pattern 3 additional:** @@ -101,8 +101,8 @@ Step 0 (seed check) is already done. Confirm the following before touching code: - [ ] Export `OLDEST_YEAR` / `LATEST_YEAR` constants (module-level, before `prepareContestProviderPresets`) so tests can assert `getSize() === LATEST - OLDEST + 1` - [ ] Pass the parameter as `section` in `super(contestType, String(param))` → provider key becomes `TYPE::value` (unique per instance) -- [ ] If `generateTable` is overridden to key the table by `task_table_index` directly: also override `getHeaderIdsForTask` using the same field and sort order -- [ ] If display title needs transformation (e.g. prepend "A. "): do it inside `generateTable`; DB data must remain unchanged +- [ ] If `task_table_index` is a numeric string key: override `getHeaderIdsForTask` with numeric sort (`Number(a) - Number(b)`) +- [ ] If display title needs transformation (e.g. prepend "A. "): override `getTaskLabels` to return `{ [contestId]: { index: letter } }`; do NOT mutate title in `generateTable` - [ ] Write override map (`Record>`) for known edge cases; test the override path by mutating the export in `beforeEach` and cleaning up in `afterEach` - [ ] If provider headings need non-default font/weight/gap: return `titleStyle` (`headingTag` / `fontSize` / `fontWeight` / `bottomGap`) from `getMetadata()`; include all set fields in the `titleStyle` assertion - [ ] `pnpm test:unit ` — **expect GREEN** diff --git a/docs/guides/how-to-add-contest-table-provider.md b/docs/guides/how-to-add-contest-table-provider.md index 53e2e2529..17548bd3a 100644 --- a/docs/guides/how-to-add-contest-table-provider.md +++ b/docs/guides/how-to-add-contest-table-provider.md @@ -228,8 +228,7 @@ for (let year = ICPC_PRELIM_LATEST_YEAR; year >= ICPC_PRELIM_OLDEST_YEAR; year-- **注意**: -- `generateTable` をオーバーライドして `task_table_index` をキーにする場合、`getHeaderIdsForTask` も**必ず同じフィールド・同じソート順**でオーバーライドすること。ベースクラスの `getHeaderIdsForTask` は `getTaskTableHeaderName()` 経由でキーを導出するため、`generateTable` と一致しないとセルが表示されない。 -- `task_table_index` が数値文字列（例: `'1664'`）の場合、辞書順ではなく数値昇順ソートが必要: `Number(a) - Number(b)` +- `task_table_index` が数値文字列キー（例: `'1664'`）の場合、ベースクラスの `getHeaderIdsForTask` は辞書順ソートになるため、**必ず数値昇順ソートでオーバーライド**すること。`generateTable`（ベースクラスが `getTaskTableHeaderName()` 経由で同じキーを使う）との一致が崩れるとセルが表示されない。`generateTable` 自体のオーバーライドは不要。 - アルゴリズムが成立しない例外年度には上書き Map を用意する: ```typescript @@ -248,6 +247,12 @@ afterEach(() => { }); ``` +- **表示ラベルは `getTaskLabels` で返し、`generateTable` ではタイトルを変更しない**。 + `generateTable` で `{ ...taskResult, title: \`${letter}. ${title}\` }` のような整形をすると、 + optimistic update で整形済みオブジェクトがソース配列に書き戻され、次の `$derived`再計算で +累積する（Issue [#3636](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/issues/3636)）。 +位置ラベルは`getTaskLabels(filtered)`が`{ [contestId]: { index: letter } }`を返し、`TaskTableBodyCell`の`$derived displayTitle`で`formatAojIcpcTitle` を呼ぶ設計にすること。 + - グループ全体に一度だけ大見出し（h2）を表示したい場合は、グループ登録時に `mainTitle: 'XXX'` を追加する。省略すると描画されない。個々の provider の `title` が冗長になるなら年や回だけに絞っても良い（ICPC 国内予選は敢えて重複させた）。 - provider 見出しのフォント・太字・余白をデフォルトから変えたい場合は `getMetadata()` で `titleStyle` を返す。`ContestTableTitleStyle`（`headingTag` / `fontSize` / `fontWeight` / `bottomGap`）のうち必要なフィールドだけ指定すればよい。 diff --git a/src/features/tasks/components/contest-table/TaskTable.svelte b/src/features/tasks/components/contest-table/TaskTable.svelte index 620cfb33d..ae9dee2d6 100644 --- a/src/features/tasks/components/contest-table/TaskTable.svelte +++ b/src/features/tasks/components/contest-table/TaskTable.svelte @@ -65,6 +65,7 @@ contestIds: Array; metadata: ContestTableMetaData; displayConfig: ContestTableDisplayConfig; + taskLabels: Record>; } let contestTableMaps = $derived(prepareContestTablesMap(providers)); @@ -98,6 +99,7 @@ contestIds: provider.getContestRoundIds(filteredTaskResults), metadata: provider.getMetadata(), displayConfig: provider.getDisplayConfig(), + taskLabels: provider.getTaskLabels(filteredTaskResults), }; } @@ -284,6 +286,7 @@ {#each contestTable.headerIds as taskTableHeaderId (taskTableHeaderId)} {@const taskResult = contestTable.innerTaskTable[contestId][taskTableHeaderId]} + {@const taskLabel = contestTable.taskLabels[contestId]?.[taskTableHeaderId]} handleUpdateTaskResult(updatedTask)} /> {/if} diff --git a/src/features/tasks/components/contest-table/TaskTableBodyCell.svelte b/src/features/tasks/components/contest-table/TaskTableBodyCell.svelte index 7d3163ccc..25afebfd4 100644 --- a/src/features/tasks/components/contest-table/TaskTableBodyCell.svelte +++ b/src/features/tasks/components/contest-table/TaskTableBodyCell.svelte @@ -7,12 +7,14 @@ import UpdatingDropdown from '$lib/components/SubmissionStatus/UpdatingDropdown.svelte'; import { getTaskUrl, removeTaskIndexFromTitle } from '$lib/utils/task'; + import { formatAojIcpcTitle } from '$features/tasks/utils/contest-table/aoj_icpc_labels'; interface Props { taskResult: TaskResult; isLoggedIn: boolean; isAtCoderVerified: boolean; isShownTaskIndex: boolean; + taskLabel?: string; voteResults: VoteStatisticsMap; onupdate?: (updatedTask: TaskResult) => void; // Ensure to update task result in parent component. } @@ -22,11 +24,23 @@ isLoggedIn, isAtCoderVerified, isShownTaskIndex, + taskLabel, voteResults, onupdate = () => {}, }: Props = $props(); let estimatedGrade = $derived(voteResults.get(taskResult.task_id)?.grade); + let displayTitle = $derived.by(() => { + if (taskLabel) { + return formatAojIcpcTitle(taskResult.title, taskLabel); + } + + if (isShownTaskIndex) { + return taskResult.title; + } + + return removeTaskIndexFromTitle(taskResult.title, taskResult.task_table_index); + });
- {@render taskTitleAndExternalLink(taskResult, isShownTaskIndex)} + {@render taskTitleAndExternalLink(taskResult)} {@render submissionUpdaterAndLinksOfTaskDetailPage(taskResult)}
@@ -52,13 +66,11 @@ /> {/snippet} -{#snippet taskTitleAndExternalLink(taskResult: TaskResult, isShownTaskIndex: boolean)} +{#snippet taskTitleAndExternalLink(taskResult: TaskResult)}
>} Nested label map, or `{}` when unused + */ + getTaskLabels(filteredTaskResults: TaskResults): Record>; } /** diff --git a/src/features/tasks/utils/contest-table/aoj_icpc_labels.test.ts b/src/features/tasks/utils/contest-table/aoj_icpc_labels.test.ts index 5e746851d..3cd08a0cd 100644 --- a/src/features/tasks/utils/contest-table/aoj_icpc_labels.test.ts +++ b/src/features/tasks/utils/contest-table/aoj_icpc_labels.test.ts @@ -1,6 +1,16 @@ import { describe, test, expect, beforeEach, afterEach } from 'vitest'; -import { buildAojIcpcLetterMap, ICPC_PRELIM_LABEL_OVERRIDES } from './aoj_icpc_labels'; +import { + buildAojIcpcLetterMap, + formatAojIcpcTitle, + ICPC_PRELIM_LABEL_OVERRIDES, +} from './aoj_icpc_labels'; + +describe('formatAojIcpcTitle', () => { + test('prepends the letter and a dot to the title', () => { + expect(formatAojIcpcTitle('Amidakuji', 'B')).toBe('B. Amidakuji'); + }); +}); describe('buildAojIcpcLetterMap', () => { test('sorts indices numerically ascending and assigns letters A, B, C...', () => { diff --git a/src/features/tasks/utils/contest-table/aoj_icpc_labels.ts b/src/features/tasks/utils/contest-table/aoj_icpc_labels.ts index 97ce21e12..20ab1d17c 100644 --- a/src/features/tasks/utils/contest-table/aoj_icpc_labels.ts +++ b/src/features/tasks/utils/contest-table/aoj_icpc_labels.ts @@ -1,6 +1,11 @@ // contest_id -> (task_table_index -> letter). Used only for years with judge gaps. export const ICPC_PRELIM_LABEL_OVERRIDES: Record> = {}; +// Prepend the assigned positional letter to an ICPC title for inline display (e.g. "A. name"). +export function formatAojIcpcTitle(title: string, letter: string): string { + return `${letter}. ${title}`; +} + // Build task_table_index -> letter map for one contest. // Default: sort indices numerically asc, assign A, B, C... // Override: if ICPC_PRELIM_LABEL_OVERRIDES[contestId] exists, use it. diff --git a/src/features/tasks/utils/contest-table/aoj_icpc_providers.test.ts b/src/features/tasks/utils/contest-table/aoj_icpc_providers.test.ts index b877d1860..b16361130 100644 --- a/src/features/tasks/utils/contest-table/aoj_icpc_providers.test.ts +++ b/src/features/tasks/utils/contest-table/aoj_icpc_providers.test.ts @@ -180,19 +180,27 @@ describe('AojIcpcPrelimProvider', () => { describe('generateTable', () => { describe('successful cases', () => { - test('assigns letter prefix A–H to all 8 titles in numeric ID order', () => { + test('stores raw titles (no letter prefix) for all 8 tasks', () => { const table = provider2023.generateTable(tasks2023); expect(table['ICPCPrelim2023']['1664'].title).toBe( - 'A. Which Team Should Receive the Sponsor Prize?', + 'Which Team Should Receive the Sponsor Prize?', ); - expect(table['ICPCPrelim2023']['1665'].title).toBe('B. Amidakuji'); - expect(table['ICPCPrelim2023']['1666'].title).toBe('C. Changing the Sitting Arrangement'); - expect(table['ICPCPrelim2023']['1667'].title).toBe('D. Efficient Problem Set'); - expect(table['ICPCPrelim2023']['1668'].title).toBe('E. Tampered Records'); - expect(table['ICPCPrelim2023']['1669'].title).toBe('F. Villa of Emblem Shape'); - expect(table['ICPCPrelim2023']['1670'].title).toBe('G. Fair Deal of Dice'); - expect(table['ICPCPrelim2023']['1671'].title).toBe('H. Planning Locations of Bus Stops'); + expect(table['ICPCPrelim2023']['1665'].title).toBe('Amidakuji'); + expect(table['ICPCPrelim2023']['1666'].title).toBe('Changing the Sitting Arrangement'); + expect(table['ICPCPrelim2023']['1667'].title).toBe('Efficient Problem Set'); + expect(table['ICPCPrelim2023']['1668'].title).toBe('Tampered Records'); + expect(table['ICPCPrelim2023']['1669'].title).toBe('Villa of Emblem Shape'); + expect(table['ICPCPrelim2023']['1670'].title).toBe('Fair Deal of Dice'); + expect(table['ICPCPrelim2023']['1671'].title).toBe('Planning Locations of Bus Stops'); + }); + + test('title is unchanged when generateTable is called twice (structurally idempotent)', () => { + const firstTable = provider2023.generateTable(tasks2023); + const secondInput = Object.values(firstTable['ICPCPrelim2023']) as TaskResults; + const secondTable = provider2023.generateTable(secondInput); + + expect(secondTable['ICPCPrelim2023']['1665'].title).toBe('Amidakuji'); }); test('uses task_table_index as the inner key', () => { @@ -216,14 +224,6 @@ describe('AojIcpcPrelimProvider', () => { expect(tasks2023[0].title).toBe(originalTitle); }); }); - - describe('edge cases', () => { - test('returns empty inner object when given empty input', () => { - const table = provider2023.generateTable([] as TaskResults); - - expect(table).toEqual({ ICPCPrelim2023: {} }); - }); - }); }); describe('getMetadata', () => { @@ -325,14 +325,14 @@ describe('AojIcpcPrelimProvider', () => { expect(provider1998.getMetadata().abbreviationName).toBe('icpcPrelim1998'); }); - test('oldest year 1998 assigns letters A–D', () => { + test('oldest year 1998 stores raw titles for 4 tasks', () => { const table = provider1998.generateTable(tasks1998); - expect(table['ICPCPrelim1998']['1100'].title).toBe('A. Area of Polygons'); - expect(table['ICPCPrelim1998']['1101'].title).toBe('B. A Simple Offline Text Editor'); - expect(table['ICPCPrelim1998']['1102'].title).toBe('C. Calculation of Expressions'); + expect(table['ICPCPrelim1998']['1100'].title).toBe('Area of Polygons'); + expect(table['ICPCPrelim1998']['1101'].title).toBe('A Simple Offline Text Editor'); + expect(table['ICPCPrelim1998']['1102'].title).toBe('Calculation of Expressions'); expect(table['ICPCPrelim1998']['1103'].title).toBe( - 'D. Board Arrangements for Concentration Games', + 'Board Arrangements for Concentration Games', ); }); @@ -349,11 +349,11 @@ describe('AojIcpcPrelimProvider', () => { expect(provider2025.getMetadata().abbreviationName).toBe('icpcPrelim2025'); }); - test('latest year 2025 assigns letters A–I (maximum problem count)', () => { + test('latest year 2025 stores raw titles (maximum problem count)', () => { const table = provider2025.generateTable(tasks2025); - expect(table['ICPCPrelim2025']['1681'].title).toBe('A. 2025'); - expect(table['ICPCPrelim2025']['1689'].title).toBe('I. Preparing the Lunch'); + expect(table['ICPCPrelim2025']['1681'].title).toBe('2025'); + expect(table['ICPCPrelim2025']['1689'].title).toBe('Preparing the Lunch'); }); test('latest year 2025 filter isolates its own contest_id', () => { @@ -385,12 +385,82 @@ describe('AojIcpcPrelimProvider', () => { delete ICPC_PRELIM_LABEL_OVERRIDES[TEST_CONTEST_ID]; }); - test('uses override map when ICPC_PRELIM_LABEL_OVERRIDES has an entry for the contest', () => { + test('generateTable stores raw titles even when override map is active', () => { const provider = createProvider(TEST_YEAR); const table = provider.generateTable(overrideTasks); - expect(table[TEST_CONTEST_ID]['9001'].title).toBe('X. Task One'); - expect(table[TEST_CONTEST_ID]['9002'].title).toBe('Y. Task Two'); + expect(table[TEST_CONTEST_ID]['9001'].title).toBe('Task One'); + expect(table[TEST_CONTEST_ID]['9002'].title).toBe('Task Two'); + }); + }); + + describe('getTaskLabels', () => { + describe('successful cases', () => { + test('returns letter map for all 8 tasks in numeric ID order (A–H)', () => { + const labels = provider2023.getTaskLabels(tasks2023); + + expect(labels['ICPCPrelim2023']['1664']).toBe('A'); + expect(labels['ICPCPrelim2023']['1665']).toBe('B'); + expect(labels['ICPCPrelim2023']['1666']).toBe('C'); + expect(labels['ICPCPrelim2023']['1667']).toBe('D'); + expect(labels['ICPCPrelim2023']['1668']).toBe('E'); + expect(labels['ICPCPrelim2023']['1669']).toBe('F'); + expect(labels['ICPCPrelim2023']['1670']).toBe('G'); + expect(labels['ICPCPrelim2023']['1671']).toBe('H'); + }); + + test('returns object keyed by contestId', () => { + const labels = provider2023.getTaskLabels(tasks2023); + + expect(Object.keys(labels)).toEqual(['ICPCPrelim2023']); + }); + }); + + describe('edge cases', () => { + test('returns empty inner object for empty input', () => { + const labels = provider2023.getTaskLabels([] as TaskResults); + + expect(labels).toEqual({ ICPCPrelim2023: {} }); + }); + }); + + describe('override map path', () => { + const TEST_YEAR = 9999; + const TEST_CONTEST_ID = `ICPCPrelim${TEST_YEAR}`; + + const overrideTasks: TaskResults = [ + { + contest_id: TEST_CONTEST_ID, + task_id: '9001', + task_table_index: '9001', + title: 'Task One', + }, + { + contest_id: TEST_CONTEST_ID, + task_id: '9002', + task_table_index: '9002', + title: 'Task Two', + }, + ] as TaskResults; + + beforeEach(() => { + ICPC_PRELIM_LABEL_OVERRIDES[TEST_CONTEST_ID] = { + '9001': 'X', + '9002': 'Y', + }; + }); + + afterEach(() => { + delete ICPC_PRELIM_LABEL_OVERRIDES[TEST_CONTEST_ID]; + }); + + test('returns custom labels from ICPC_PRELIM_LABEL_OVERRIDES', () => { + const provider = createProvider(TEST_YEAR); + const labels = provider.getTaskLabels(overrideTasks); + + expect(labels[TEST_CONTEST_ID]['9001']).toBe('X'); + expect(labels[TEST_CONTEST_ID]['9002']).toBe('Y'); + }); }); }); }); diff --git a/src/features/tasks/utils/contest-table/aoj_icpc_providers.ts b/src/features/tasks/utils/contest-table/aoj_icpc_providers.ts index b90a42f51..422e442e7 100644 --- a/src/features/tasks/utils/contest-table/aoj_icpc_providers.ts +++ b/src/features/tasks/utils/contest-table/aoj_icpc_providers.ts @@ -3,7 +3,6 @@ import type { TaskResult, TaskResults } from '$lib/types/task'; import { type ContestTableMetaData, type ContestTableDisplayConfig, - type ContestTable, } from '$features/tasks/types/contest-table/contest_table_provider'; import { ContestTableProviderBase } from './contest_table_provider_base'; @@ -23,24 +22,6 @@ export class AojIcpcPrelimProvider extends ContestTableProviderBase { return (taskResult: TaskResult) => taskResult.contest_id === this.contestId; } - // Override: prepend assigned letter to the title (display only; DB unchanged). - // ICPC titles are stored as "{name}" only, so no prefix removal is needed. - generateTable(filtered: TaskResults): ContestTable { - const letterMap = buildAojIcpcLetterMap( - this.contestId, - filtered.map((taskResult) => taskResult.task_table_index), - ); - const table: ContestTable = { [this.contestId]: {} }; - - for (const taskResult of filtered) { - const index = taskResult.task_table_index; - const letter = letterMap.get(index) ?? index; - table[this.contestId][index] = { ...taskResult, title: `${letter}. ${taskResult.title}` }; - } - - return table; - } - // Ensure left-to-right cell order is numeric (A,B,C...). Safeguard for variable-width ids. getHeaderIdsForTask(filtered: TaskResults): string[] { return Array.from(new Set(filtered.map((taskResult) => taskResult.task_table_index))).sort( @@ -74,4 +55,13 @@ export class AojIcpcPrelimProvider extends ContestTableProviderBase { getContestRoundLabel(_contestId: string): string { return `ICPC 国内予選 ${this.year}`; } + + override getTaskLabels(filtered: TaskResults): Record> { + const letterMap = buildAojIcpcLetterMap( + this.contestId, + filtered.map((taskResult) => taskResult.task_table_index), + ); + + return { [this.contestId]: Object.fromEntries(letterMap) }; + } } diff --git a/src/features/tasks/utils/contest-table/contest_table_provider_base.test.ts b/src/features/tasks/utils/contest-table/contest_table_provider_base.test.ts index cea4b994b..a24d4b8c5 100644 --- a/src/features/tasks/utils/contest-table/contest_table_provider_base.test.ts +++ b/src/features/tasks/utils/contest-table/contest_table_provider_base.test.ts @@ -23,5 +23,18 @@ describe('ContestTableProviderBase', () => { expect(headerIds).toEqual(['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K']); }); + + test('getTaskLabels returns empty object by default', () => { + const provider = new ABSProvider(ContestType.ABS); + + expect(provider.getTaskLabels([])).toEqual({}); + }); + + test('getTaskLabels returns empty object regardless of input', () => { + const provider = new ABSProvider(ContestType.ABS); + const nonEmpty = taskResultsForABS.filter((task) => task.contest_id === 'abs'); + + expect(provider.getTaskLabels(nonEmpty)).toEqual({}); + }); }); }); diff --git a/src/features/tasks/utils/contest-table/contest_table_provider_base.ts b/src/features/tasks/utils/contest-table/contest_table_provider_base.ts index ba6831599..ef41ae617 100644 --- a/src/features/tasks/utils/contest-table/contest_table_provider_base.ts +++ b/src/features/tasks/utils/contest-table/contest_table_provider_base.ts @@ -111,6 +111,10 @@ export abstract class ContestTableProviderBase implements ContestTableProvider { } abstract getContestRoundLabel(contestId: string): string; + + getTaskLabels(_filteredTaskResults: TaskResults): Record> { + return {}; + } } export function parseContestRound(contestId: string, prefix: string): number {