Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ritable-derived in eslint-plugin-svelte update plan Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add keyed {#each} blocks (svelte/require-each-key)
- Replace hardcoded hrefs and goto() calls with resolve() ($app/paths)
- Convert Map to SvelteMap for reactive updates
- Replace $state+$effect patterns with $derived (prefer-writable-derived)
- Add rel="noreferrer external" to external links
- Remove unused props (status in users/edit, formAction in account_transfer)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add resolve() and SvelteMap patterns to coding-style.md rules - Update svelte-components.md with SvelteMap and each-key guidelines - Remove phase-1..8 files (consolidated into plan.md) - Add refactor.md with lessons learned and CodeRabbit findings - Update plan.md checklist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ule files - Extract Svelte Runes patterns into new .claude/rules/svelte-runes.md - Extract SvelteKit patterns into new .claude/rules/sveltekit.md - Remove extracted content from coding-style.md and svelte-components.md - Add Prisma relation filter note to prisma-db.md - Simplify AGENTS.md refactor cycle instructions (consolidate into plan.md) - Consolidate refactor.md findings into plan.md and remove refactor.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthrougheslint-plugin-svelte v3.16.0へのアップグレード対応として、Svelte 5のreactivityルール（ Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/components/SubmissionStatus/UpdatingDropdown.svelte (1)
214-216:⚠️ Potential issue | 🔴 Critical
resolve()でラップが必要プロジェクトの ESLint ルール (
svelte/no-navigation-without-resolve) により、内部ナビゲーションにはresolve()が必須です。/signupと/loginは静的なルートのため、<script>ブロック内で事前計算し、テンプレートから参照する必要があります:// `@ts-expect-error` svelte-check TS2554: AppTypes declaration merging causes RouteId to resolve as string, requiring params. Runtime behavior is correct. const signupHref = resolve('/signup'); // `@ts-expect-error` svelte-check TS2554: AppTypes declaration merging causes RouteId to resolve as string, requiring params. Runtime behavior is correct. const loginHref = resolve('/login');その後、テンプレートで使用:
<DropdownItem href={signupHref} class="rounded-md">アカウント作成</DropdownItem> <DropdownItem href={loginHref} class="rounded-md">ログイン</DropdownItem>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/SubmissionStatus/UpdatingDropdown.svelte` around lines 214 - 216, ESLint rule svelte/no-navigation-without-resolve requires internal navigation to use resolve(); replace direct href usage in the template (currently DropdownItem href={SIGNUP_PAGE} and href={LOGIN_PAGE}) by precomputing the resolved URLs in the <script> block as signupHref and loginHref using resolve('/signup') and resolve('/login') respectively (add the suggested // `@ts-expect-error` comments to silence svelte-check TS2554), then update the template to use <DropdownItem href={signupHref} ...> and <DropdownItem href={loginHref} ...>.src/lib/components/TaskGradeList.svelte (1)
19-31: 🛠️ Refactor suggestion | 🟠 Major
run()から抜け出し、ロジックを utils に切り出してください
run()はsvelte/legacyの deprecated API です。grade ごとのフィルタリング処理は utils にgroupTaskResultsByGrade(taskResults)として抽出し、コンポーネント側は$derivedで受け取る形に変更してください。コーディングガイドラインの「Svelte 5 Runes を使用する」「ビジネスロジックは utils に配置する」に準拠するためです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/TaskGradeList.svelte` around lines 19 - 31, Replace the run() block by extracting the grade-grouping logic into a new utility function groupTaskResultsByGrade(taskResults) that returns a Map<TaskGrade, TaskResults> (used to compute per-grade arrays from the taskResults array); remove usage of run() and taskResultsForEachGrade mutation, then in TaskGradeList.svelte create a derived store (using $derived) that calls groupTaskResultsByGrade($taskResults) and expose its result to the component; update references to taskResultsForEachGrade, taskGradeValues and taskResults to consume the derived store instead of mutating a SvelteMap inside run().src/features/tasks/components/contest-table/TaskTable.svelte (1)
66-80:⚠️ Potential issue | 🟠 Major
$derived(() => ...)では呼び出すたびに新しいSvelteMapが生成されます
$derived(expression)と$derived.by(fn)は異なるセマンティクスを持ちます。ここで$derived(() => ...)として式の中に関数定義を含めると、その関数が毎回呼び出されて新しいSvelteMapが作られます。そのため、handleUpdateTaskResult内のmap.set(...)は一時的に作られたマップにのみ反映され、永続化されません。
$derived.by(() => prepareContestTablesMap(providers))に変更し、呼び出し側のtaskResultsMap()やtaskIndicesMap()から()を削除してください。修正イメージ
- let contestTableMaps = $derived(() => prepareContestTablesMap(providers)); + let contestTableMaps = $derived.by(() => prepareContestTablesMap(providers)); - let taskResultsMap = $derived(() => { + let taskResultsMap = $derived.by(() => { return taskResults.reduce( (map: SvelteMap<ContestTaskPairKey, TaskResult>, taskResult: TaskResult) => { const key = createContestTaskPairKey(taskResult.contest_id, taskResult.task_id); if (!map.has(key)) { map.set(key, taskResult); } return map; }, new SvelteMap<ContestTaskPairKey, TaskResult>(), ); }); - let taskIndicesMap = $derived(() => { + let taskIndicesMap = $derived.by(() => { const indices = new Map<ContestTaskPairKey, number>(); taskResults.forEach((task, index) => { const key = createContestTaskPairKey(task.contest_id, task.task_id); indices.set(key, index); }); return indices; });呼び出し側も変更:
taskResultsMap()→taskResultsMap、taskIndicesMap()→taskIndicesMapAlso applies to: 149-160, 175-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/tasks/components/contest-table/TaskTable.svelte` around lines 66 - 80, contestTableMaps is currently created with $derived(() => prepareContestTablesMap(providers)), which recreates a new SvelteMap on every call so mutations in handleUpdateTaskResult aren't persisted; change the declaration to use $derived.by(() => prepareContestTablesMap(providers)) so the map instance is stable, and update all callers to access the derived value without invoking it (e.g., use taskResultsMap and taskIndicesMap instead of taskResultsMap() and taskIndicesMap()); also apply the same $derived.by change to the other derived map usages corresponding to the later prepare* functions so their callers stop using () as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-notes/2026-03-22/eslint-plugin-svelte-update/plan.md`:
- Around line 52-58: The document mixes overall completion with remaining
work—clarify by splitting the “全 8 フェーズ完了” statement from the “残課題” section:
explicitly state which scope is complete (e.g., "lint対応フェーズは完了") and mark
AtCoder re-publication as a separate pending task; in the AtCoder note reference
the file users/edit/+page.svelte, the commented-out AtCoder ID form, the removed
status prop and the valid-prop-names-in-kit-pages constraint, and add a short
action item to re-evaluate prop design (e.g., consolidate to data prop) when
re-enabling the feature.
In `@src/features/workbooks/components/shared/WorkbookLink.svelte`:
- Line 13: WorkbookLink currently builds URLs from workBookId
(href={resolve('/workbooks/[slug]', { slug: String(workBookId) })}) while
TitleTableBodyCell uses getUrlSlugFrom() on a full workbook, causing
inconsistent URL sources; update WorkbookLink to accept an optional urlSlug (or
a workbook object) and prefer that when present, falling back to
String(workBookId) otherwise, and adjust call sites (e.g., where
TitleTableBodyCell or KanbanCard construct links) to pass
getUrlSlugFrom(workbook) when available so both components resolve the same slug
consistently; reference symbols: WorkbookLink, workBookId, getUrlSlugFrom(),
TitleTableBodyCell, KanbanCard, Card.
In `@src/lib/components/AuthForm.svelte`:
- Around line 45-50: The prop alternativePageLink should be typed as the union
'/login' | '/signup' instead of string to ensure type safety; update the
component's props/type declaration for alternativePageLink accordingly, remove
the manual cast to '/login' on the $derived(resolve(...)) call at the
alternativeHref assignment, and delete the associated `@ts-expect-error` comment
so TypeScript will enforce valid routes; locate symbols alternativePageLink,
alternativeHref, resolve, and the $derived usage in AuthForm.svelte to make
these changes.
In `@src/lib/components/TagForm.svelte`:
- Around line 32-33: Remove the leading ts-expect-error suppression and change
the static resolve call used to build tagsHref: replace the line that declares
const tagsHref = resolve('/(admin)/tags') (and remove the preceding "//
`@ts-expect-error` ..." comment) with a call using the pathname form
resolve('/tags') so base-path resolution is preserved; that is, update the
tagsHref initialization to use resolve('/tags') and delete the `@ts-expect-error`
comment.
In `@src/routes/`(admin)/account_transfer/+page.svelte:
- Around line 31-32: The form action value is using $state('account_transfer')
and missing the required SvelteKit named-action prefix and mutability: replace
usage of $state and the bare name with a constant string using the named-action
format `?/account_transfer` and assign it to a const (e.g., change formAction to
a const named formAction set to "?/account_transfer") so the form posts to the
correct named action; update any references to the previous formAction variable
accordingly.
In `@src/routes/`(admin)/workbooks/order/_components/KanbanBoard.svelte:
- Line 4: buildUpdatedUrl() already returns a URL object, so calling
resolve(updatedUrl.pathname + updatedUrl.search) causes an unnecessary string
conversion and TypeScript errors; in KanbanBoard.svelte replace usages of
resolve(...) with the URL object (e.g. use updatedUrl.href or
updatedUrl.pathname + updatedUrl.search typed as string) or pass the URL
directly where a URL is accepted, and remove the import { resolve } from
'$app/paths' if no longer used to avoid the TS error.
In `@src/routes/users/`[username]/+page.svelte:
- Around line 16-17: PROBLEMS_PAGE is a static path so remove the unnecessary
resolve() call and the accompanying ts-expect-error; update the declaration that
currently uses resolve(PROBLEMS_PAGE) to use PROBLEMS_PAGE directly (i.e. change
the const problemsHref assignment to use PROBLEMS_PAGE) and delete the preceding
"// `@ts-expect-error` ..." comment so the code no longer depends on resolve or
the type override.
In `@src/routes/workbooks/`+page.svelte:
- Around line 52-53: The code is passing a full path with query params (from
buildWorkbooksUrl) into SvelteKit's resolve(), which is for route IDs and breaks
navigation; update the calls (e.g., the goto(resolve(saved), { replaceState:
true }) at the saved usage and the similar sites referenced at 71-72, 76-77,
81-82) to call goto with the already-built URL directly (e.g., goto(saved, {
replaceState: true }) or goto(buildWorkbooksUrl(...), { replaceState: true }))
and remove the unnecessary resolve() wrapping.
---
Outside diff comments:
In `@src/features/tasks/components/contest-table/TaskTable.svelte`:
- Around line 66-80: contestTableMaps is currently created with $derived(() =>
prepareContestTablesMap(providers)), which recreates a new SvelteMap on every
call so mutations in handleUpdateTaskResult aren't persisted; change the
declaration to use $derived.by(() => prepareContestTablesMap(providers)) so the
map instance is stable, and update all callers to access the derived value
without invoking it (e.g., use taskResultsMap and taskIndicesMap instead of
taskResultsMap() and taskIndicesMap()); also apply the same $derived.by change
to the other derived map usages corresponding to the later prepare* functions so
their callers stop using () as well.
In `@src/lib/components/SubmissionStatus/UpdatingDropdown.svelte`:
- Around line 214-216: ESLint rule svelte/no-navigation-without-resolve requires
internal navigation to use resolve(); replace direct href usage in the template
(currently DropdownItem href={SIGNUP_PAGE} and href={LOGIN_PAGE}) by
precomputing the resolved URLs in the <script> block as signupHref and loginHref
using resolve('/signup') and resolve('/login') respectively (add the suggested
// `@ts-expect-error` comments to silence svelte-check TS2554), then update the
template to use <DropdownItem href={signupHref} ...> and <DropdownItem
href={loginHref} ...>.
In `@src/lib/components/TaskGradeList.svelte`:
- Around line 19-31: Replace the run() block by extracting the grade-grouping
logic into a new utility function groupTaskResultsByGrade(taskResults) that
returns a Map<TaskGrade, TaskResults> (used to compute per-grade arrays from the
taskResults array); remove usage of run() and taskResultsForEachGrade mutation,
then in TaskGradeList.svelte create a derived store (using $derived) that calls
groupTaskResultsByGrade($taskResults) and expose its result to the component;
update references to taskResultsForEachGrade, taskGradeValues and taskResults to
consume the derived store instead of mutating a SvelteMap inside run().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ea06b83-3efc-4353-8ed5-35cfd03afefc
📒 Files selected for processing (35)
.claude/rules/coding-style.md.claude/rules/prisma-db.md.claude/rules/svelte-components.md.claude/rules/svelte-runes.md.claude/rules/sveltekit.mdAGENTS.mddocs/dev-notes/2026-03-22/eslint-plugin-svelte-update/plan.mdsrc/features/tasks/components/contest-table/TaskTable.sveltesrc/features/workbooks/components/detail/WorkBookTasksTable.sveltesrc/features/workbooks/components/list/TitleTableBodyCell.sveltesrc/features/workbooks/components/list/WorkbookAuthorActionsCell.sveltesrc/features/workbooks/components/shared/WorkbookLink.sveltesrc/lib/components/AuthForm.sveltesrc/lib/components/ExternalLinkWrapper.sveltesrc/lib/components/Header.sveltesrc/lib/components/LabelWithTooltips.sveltesrc/lib/components/SubmissionStatus/UpdatingDropdown.sveltesrc/lib/components/TagForm.sveltesrc/lib/components/TagListForEdit.sveltesrc/lib/components/TaskGradeList.sveltesrc/lib/components/TaskGrades/GradeGuidelineTable.sveltesrc/lib/components/TaskList.sveltesrc/lib/components/TaskListForEdit.sveltesrc/lib/components/TaskListSorted.sveltesrc/lib/components/TaskSearchBox.sveltesrc/lib/components/ThermometerProgressBar.sveltesrc/routes/(admin)/account_transfer/+page.sveltesrc/routes/(admin)/workbooks/order/_components/ColumnSelector.sveltesrc/routes/(admin)/workbooks/order/_components/KanbanBoard.sveltesrc/routes/about/SectionSnippets.sveltesrc/routes/problems/[slug]/+page.sveltesrc/routes/users/[username]/+page.sveltesrc/routes/users/edit/+page.sveltesrc/routes/workbooks/+page.sveltesrc/routes/workbooks/[slug]/+page.svelte
💤 Files with no reviewable changes (1)
- src/routes/users/edit/+page.svelte
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- TaskTable: $derived(() => fn) → $derived.by() to correctly track reactivity - account_transfer: formAction string → '?/account_transfer' with proper prefix - workbooks: separate path/search before resolve() to avoid query string issue - KanbanBoard: remove resolve() on $page.url-derived path (avoids double base path) - AuthForm: narrow alternativePageLink type to '/login' | '/signup' - UpdatingDropdown: wrap SIGNUP_PAGE and LOGIN_PAGE hrefs with resolve() - TaskGradeList: replace legacy run() shim with $derived and utility function - Update svelte-runes.md and sveltekit.md rules accordingly - Update plan.md checklist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/workbooks/+page.svelte (1)
93-93:⚠️ Potential issue | 🟡 Minor
Buttonのhrefにresolve()が未適用です。Line 93 の
/workbooks/createは内部ナビゲーションですが、resolve()でラップされていません。svelte/no-navigation-without-resolveルール（eslint-plugin-svelte v3.16.0 で追加）に違反しており、base path 対応時に必要となります。修正:
href={resolve("/workbooks/create")}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/workbooks/`+page.svelte at line 93, The Button component's href is set directly to "/workbooks/create" which violates svelte/no-navigation-without-resolve; wrap the path with the router resolve function (use resolve("/workbooks/create")) so internal navigation respects base paths — update the href prop on the Button element (Button) to call resolve(...) instead of using the raw string.src/features/tasks/components/contest-table/TaskTable.svelte (2)
164-173:⚠️ Potential issue | 🟠 Major同様に
$derived.by()を使用すべき
taskIndicesMapも同じパターン。🐛 修正案
- let taskIndicesMap = $derived(() => { + let taskIndicesMap = $derived.by(() => { const indices = new Map<ContestTaskPairKey, number>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/tasks/components/contest-table/TaskTable.svelte` around lines 164 - 173, taskIndicesMap is created using $derived(() => ...) but should use $derived.by for the indexed-lookup pattern; update the creation of taskIndicesMap to use $derived.by with taskResults as the source and build the Map inside the per-item callback, referencing createContestTaskPairKey and taskResults so each task and its index are used to set the Map entry (preserve the Map<ContestTaskPairKey, number> return type and behavior).
149-162:⚠️ Potential issue | 🟠 MajorSvelte 5 の
$derived.by()を使用すべき箇所PR目的で
$derived(() => fn) → $derived.by()への移行が挙げられているが、このコンポーネントではまだ関数本体を持つ$derived(() => { return ... })パターンが残っている。Svelte 5では、ブロック本体による複雑なロジックには$derived.by()を使用する必要がある。修正案
- let taskResultsMap = $derived(() => { + let taskResultsMap = $derived.by(() => { return taskResults.reduce( (map: SvelteMap<ContestTaskPairKey, TaskResult>, taskResult: TaskResult) => { const key = createContestTaskPairKey(taskResult.contest_id, taskResult.task_id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/tasks/components/contest-table/TaskTable.svelte` around lines 149 - 162, The derived store taskResultsMap currently uses a block-style $derived(() => { return ... }) with reduce; replace this with Svelte 5's $derived.by(...) form to handle complex logic: move the reduce logic that builds the SvelteMap (using createContestTaskPairKey, taskResults, and SvelteMap<ContestTaskPairKey, TaskResult>) into a function passed to $derived.by so the derived store is created via $derived.by(() => { ... }) or the appropriate signature, ensuring the map population (map.has / map.set) remains identical and the returned value is the constructed SvelteMap.
♻️ Duplicate comments (1)
src/routes/workbooks/+page.svelte (1)
71-84:⚠️ Potential issue | 🔴 Critical
resolve()に query string 付きパスを渡しています。
buildWorkbooksUrl()は/workbooks?tab=curriculum&grades=Q9のような query string 付きパスを返します（context snippet 参照）。resolve()は route pattern のみ受け付けるため、これは.claude/rules/sveltekit.mdLine 35 に違反しています。Line 52-54 で適用した
pathname/search分離パターンをここでも使用してください。🐛 修正案
function handleTabChange(tab: WorkBookTab) { - // `@ts-expect-error` svelte-check TS2554: same declaration merging issue - goto(resolve(TAB_URL_BUILDERS[tab]())); + const url = new URL(TAB_URL_BUILDERS[tab](), window.location.origin); + // `@ts-expect-error` svelte-check TS2554: same declaration merging issue + goto(resolve(url.pathname) + url.search); } function handleGradeChange(grade: TaskGrade) { - // `@ts-expect-error` svelte-check TS2554: same declaration merging issue - goto(resolve(buildWorkbooksUrl(WorkBookTab.CURRICULUM, grade))); + const url = new URL(buildWorkbooksUrl(WorkBookTab.CURRICULUM, grade), window.location.origin); + // `@ts-expect-error` svelte-check TS2554: same declaration merging issue + goto(resolve(url.pathname) + url.search); } function handleCategoryChange(category: SolutionCategory) { - // `@ts-expect-error` svelte-check TS2554: same declaration merging issue - goto(resolve(buildWorkbooksUrl(WorkBookTab.SOLUTION, undefined, category))); + const url = new URL(buildWorkbooksUrl(WorkBookTab.SOLUTION, undefined, category), window.location.origin); + // `@ts-expect-error` svelte-check TS2554: same declaration merging issue + goto(resolve(url.pathname) + url.search); }または、ヘルパー関数を抽出して重複を削減できます：
function resolveWithSearch(path: string): string { const url = new URL(path, window.location.origin); // `@ts-expect-error` svelte-check TS2554: AppTypes declaration merging issue return resolve(url.pathname) + url.search; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/workbooks/`+page.svelte around lines 71 - 84, The calls in handleTabChange, handleGradeChange, and handleCategoryChange pass full URLs with query strings into resolve (which expects a route pattern); change them to split pathname and search like you did earlier: call resolve(url.pathname) and then append url.search, where url is new URL(pathReturnedFromBuildWorkbooksUrlOrTAB_URL_BUILDERS, window.location.origin), or extract this logic into a helper (e.g., resolveWithSearch) and use it in handleTabChange, handleGradeChange, and handleCategoryChange so resolve only receives the pathname and the query string is appended afterward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/tasks/components/contest-table/TaskTable.svelte`:
- Around line 68-69: The function prepareContestTablesMap currently declares its
return type as Map<string, ProviderData> but constructs and returns a
SvelteMap<string, ProviderData>; update the signature to return
SvelteMap<string, ProviderData> OR convert the SvelteMap to a plain Map before
returning (e.g., populate a new Map from the SvelteMap entries) so the declared
return type matches the actual returned value; locate the function
prepareContestTablesMap and change the type annotation or add the conversion to
ensure types are consistent.
In `@src/lib/components/SubmissionStatus/UpdatingDropdown.svelte`:
- Line 207: The {`#each` submissionStatusOptions as submissionStatus
(submissionStatus.innerName)} uses submissionStatus.innerName as the key which
can change or collide; update the each block in UpdatingDropdown.svelte to use
the stable unique identifier submissionStatus.innerId instead (i.e., change the
key expression from submissionStatus.innerName to submissionStatus.innerId) so
the DOM list uses a stable unique key for submissionStatus items.
In `@src/lib/components/TaskGradeList.svelte`:
- Line 48: Replace the non-null assertion on the Map lookup with an explicit
fallback to avoid runtime errors if the Map changes; in TaskGradeList.svelte,
change usages of taskResultsForEachGrade.get(taskGrade)! (the value passed to
the taskResults prop) to use a safe default (e.g., fallback to an empty array or
appropriate default) so taskResults receives a defined value even when
get(taskGrade) returns undefined.
In `@src/routes/`(admin)/account_transfer/+page.svelte:
- Line 31: フォームのnamed actionとサーバー側ハンドラが不一致しているため送信が失敗します：フロント側定数
ACCOUNT_TRANSFER_ACTION = '?/account_transfer' を使うなら、サーバーの +page.server.ts に
`export const actions = { account_transfer: async (event) => { ... } }` のように
account_transfer アクションを追加するか、既存の default アクションを使いたいならページのフォームから action
属性（ACCOUNT_TRANSFER_ACTION を使っている箇所）を削除して default ハンドラ（+page.server.ts の export
const actions.default）に委ねてください。
---
Outside diff comments:
In `@src/features/tasks/components/contest-table/TaskTable.svelte`:
- Around line 164-173: taskIndicesMap is created using $derived(() => ...) but
should use $derived.by for the indexed-lookup pattern; update the creation of
taskIndicesMap to use $derived.by with taskResults as the source and build the
Map inside the per-item callback, referencing createContestTaskPairKey and
taskResults so each task and its index are used to set the Map entry (preserve
the Map<ContestTaskPairKey, number> return type and behavior).
- Around line 149-162: The derived store taskResultsMap currently uses a
block-style $derived(() => { return ... }) with reduce; replace this with Svelte
5's $derived.by(...) form to handle complex logic: move the reduce logic that
builds the SvelteMap (using createContestTaskPairKey, taskResults, and
SvelteMap<ContestTaskPairKey, TaskResult>) into a function passed to $derived.by
so the derived store is created via $derived.by(() => { ... }) or the
appropriate signature, ensuring the map population (map.has / map.set) remains
identical and the returned value is the constructed SvelteMap.
In `@src/routes/workbooks/`+page.svelte:
- Line 93: The Button component's href is set directly to "/workbooks/create"
which violates svelte/no-navigation-without-resolve; wrap the path with the
router resolve function (use resolve("/workbooks/create")) so internal
navigation respects base paths — update the href prop on the Button element
(Button) to call resolve(...) instead of using the raw string.
---
Duplicate comments:
In `@src/routes/workbooks/`+page.svelte:
- Around line 71-84: The calls in handleTabChange, handleGradeChange, and
handleCategoryChange pass full URLs with query strings into resolve (which
expects a route pattern); change them to split pathname and search like you did
earlier: call resolve(url.pathname) and then append url.search, where url is new
URL(pathReturnedFromBuildWorkbooksUrlOrTAB_URL_BUILDERS,
window.location.origin), or extract this logic into a helper (e.g.,
resolveWithSearch) and use it in handleTabChange, handleGradeChange, and
handleCategoryChange so resolve only receives the pathname and the query string
is appended afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b69a47b-6eb3-4553-85ca-07597db5a354
📒 Files selected for processing (10)
.claude/rules/svelte-runes.md.claude/rules/sveltekit.mddocs/dev-notes/2026-03-22/eslint-plugin-svelte-update/plan.mdsrc/features/tasks/components/contest-table/TaskTable.sveltesrc/lib/components/AuthForm.sveltesrc/lib/components/SubmissionStatus/UpdatingDropdown.sveltesrc/lib/components/TaskGradeList.sveltesrc/routes/(admin)/account_transfer/+page.sveltesrc/routes/(admin)/workbooks/order/_components/KanbanBoard.sveltesrc/routes/workbooks/+page.svelte
- TaskTable: revert SvelteMap inside $derived to plain Map (reactivity tracked at $derived level) - account_transfer: remove named action attribute — form uses default action after PR #3298 - Update svelte-runes.md: clarify SvelteMap is unnecessary inside $derived - Update plan.md: add round-2 findings and non-adopted suggestions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/tasks/components/contest-table/TaskTable.svelte (2)
164-173:⚠️ Potential issue | 🟠 Major同様に
$derived.byを使用してください。
taskIndicesMapも$derived(() => ...)パターンを使用しており、上記と同じ問題があります。修正案
- let taskIndicesMap = $derived(() => { + let taskIndicesMap = $derived.by(() => { const indices = new Map<ContestTaskPairKey, number>(); ... return indices; });呼び出し側:
- const index = taskIndicesMap().get(key); + const index = taskIndicesMap.get(key);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/tasks/components/contest-table/TaskTable.svelte` around lines 164 - 173, taskIndicesMap is using $derived(() => ...) like the previous example and should be converted to use $derived.by to get a stable, dependency-aware derivation; replace the current definition of taskIndicesMap with a $derived.by that depends on taskResults and constructs the Map<ContestTaskPairKey, number> by iterating taskResults and using createContestTaskPairKey(task.contest_id, task.task_id) to set indices, so the derived only reruns when taskResults changes and keeps reactive correctness.
149-162:⚠️ Potential issue | 🟠 Major
$derived(() => ...)は関数オブジェクト自体を保持するバグパターンです。
$derived(() => { ... })に arrow function を渡すと、reduceの結果ではなく関数オブジェクトがtaskResultsMapに格納される。Line 177 のtaskResultsMap()呼び出しは現状動作するが、これは「derived 値が関数だから呼び出せる」だけで意図した挙動ではない。
$derived.by(() => { ... })を使用するか、単一式なら$derived(taskResults.reduce(...))に変更してください。修正案
- let taskResultsMap = $derived(() => { - return taskResults.reduce( + let taskResultsMap = $derived.by(() => { + return taskResults.reduce( (map: SvelteMap<ContestTaskPairKey, TaskResult>, taskResult: TaskResult) => { const key = createContestTaskPairKey(taskResult.contest_id, taskResult.task_id); if (!map.has(key)) { map.set(key, taskResult); } return map; }, new SvelteMap<ContestTaskPairKey, TaskResult>(), ); });呼び出し側も修正:
- const map = taskResultsMap(); + const map = taskResultsMap;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/tasks/components/contest-table/TaskTable.svelte` around lines 149 - 162, The current use of $derived with an arrow function stores the function object instead of the reduced map; change the derived creation for taskResultsMap so it returns the computed SvelteMap value (either by using $derived.by(() => { ... }) or by passing the single-expression result to $derived instead of a function) and ensure callers using taskResultsMap() are updated to access the derived value correctly (remove the extra invocation if the derived now yields the map directly); locate symbols taskResultsMap, $derived, taskResults.reduce, createContestTaskPairKey and update accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/features/tasks/components/contest-table/TaskTable.svelte`:
- Around line 164-173: taskIndicesMap is using $derived(() => ...) like the
previous example and should be converted to use $derived.by to get a stable,
dependency-aware derivation; replace the current definition of taskIndicesMap
with a $derived.by that depends on taskResults and constructs the
Map<ContestTaskPairKey, number> by iterating taskResults and using
createContestTaskPairKey(task.contest_id, task.task_id) to set indices, so the
derived only reruns when taskResults changes and keeps reactive correctness.
- Around line 149-162: The current use of $derived with an arrow function stores
the function object instead of the reduced map; change the derived creation for
taskResultsMap so it returns the computed SvelteMap value (either by using
$derived.by(() => { ... }) or by passing the single-expression result to
$derived instead of a function) and ensure callers using taskResultsMap() are
updated to access the derived value correctly (remove the extra invocation if
the derived now yields the map directly); locate symbols taskResultsMap,
$derived, taskResults.reduce, createContestTaskPairKey and update accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a2d45c5-c86f-45fa-8247-8cf6ebeadde0
📒 Files selected for processing (4)
.claude/rules/svelte-runes.mddocs/dev-notes/2026-03-22/eslint-plugin-svelte-update/plan.mdsrc/features/tasks/components/contest-table/TaskTable.sveltesrc/routes/(admin)/account_transfer/+page.svelte
- taskResultsMap and taskIndicesMap: $derived(() => fn) held a function ref instead of computing a value; $derived.by() evaluates the factory correctly - Revert SvelteMap to plain Map in reduce accumulator ($derived tracks reactivity) - Remove call-site () invocations now that derived values are accessed directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
close #2345
Summary by CodeRabbit
リリースノート
ドキュメント
バグ修正・改善