Skip to content

feat: SKILL.mdを修正 #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
takehiroman merged 1 commit into from
Mar 21, 2026
Merged

feat: SKILL.mdを修正 #41

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,39 +24,43 @@ Use this skill for feature work, bug fixes, refactors, or reviews in this reposi
- The frontend builds a typed client with `hc<AppType>('/')` and calls:
- `GET /api/weapons/random?count=<n>`
- `GET /api/results`
- `POST /api/results`
- Local Vite dev proxies `/api` to `http://localhost:3000`.
- Result history is stored in Cloudflare KV under a single bounded key (`results`) rather than one key per record.

## Working rules for this repo

- Treat `src/app.tsx` as the current source of truth for user-visible behavior.
- Keep frontend and API changes aligned. If you change an API response shape, update the typed client usage in `src/app.tsx` in the same task.
- Be careful with the boundary between real data and placeholders:
- `useSWR('results', ...)` fetches live result data from KV.
- `cards` in `src/app.tsx` are currently hardcoded sample history entries.
- Do not assume the rendered history UI is already wired to backend data.
- History is live data now:
- `useSWR('results', ...)` reads current history from `GET /api/results`.
- After `POST /api/results`, the frontend updates the SWR cache directly instead of forcing a refetch.
Comment on lines +36 to +37

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

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

src/app.tsx sets revalidateOnMount: false (and also disables focus/reconnect revalidation) and there’s no SWRConfig fallback/other mount-time trigger in src/, so the history list may not be fetched on initial page load. This makes the statement that useSWR('results', ...) “reads current history from GET /api/results” potentially misleading; consider clarifying the docs or adjusting the app’s initial fetch strategy so history is actually loaded on mount.

Suggested change
- `useSWR('results', ...)` reads current history from `GET /api/results`.
- After `POST /api/results`, the frontend updates the SWR cache directly instead of forcing a refetch.
- `useSWR('results', ...)` manages the result history cache under the `'results'` key. Given the current SWR config in `src/app.tsx` (with `revalidateOnMount`, focus, and reconnect revalidation disabled), history is not automatically fetched on initial page load unless you explicitly trigger a fetch.
- After `POST /api/results`, the frontend updates the SWR cache directly instead of forcing a refetch, to avoid extra KV reads.

Copilot uses AI. Check for mistakes.
- Keep the frontend cache update logic aligned with the backend retention limit.
- `src/constants/weapon.ts` appears to be legacy or unused. Confirm usage before editing it.
- `tsconfig.json` includes only `src`, even though the frontend imports types from `functions/`. If type changes behave strangely, inspect this include boundary first.
- `package.json` is configured for `husky` and `lint-staged`. Avoid assuming hooks are consistently installed.
- Free-tier-conscious behavior matters here. Prefer bounded KV usage and avoid extra reads when the UI can update from the mutation response.

## Safe change workflow

1. Read `src/app.tsx` and the relevant route files before editing.
2. Prefer the existing API shape and component patterns unless the task clearly requires a change.
3. For backend changes, verify the corresponding Cloudflare binding exists in `wrangler.toml`.
4. For frontend changes, check whether data is live or placeholder before wiring UI logic.
4. For history changes, update both `functions/api/routes/result.ts` and the SWR mutation logic in `src/app.tsx`.
5. Run the narrowest useful verification available after edits.

## Verification

- Install dependencies: `npm install`
- Frontend dev server: `npm run dev`
- Production build: `npm run build`
- Lint scripts: `npm run lint:script`
- Lint styles: `npm run lint:style`

If a task touches Cloudflare runtime behavior, note that local verification may also require a Pages/Workers dev setup that is not fully captured by current package scripts.

## Known pitfalls

- `functions/api/routes/weapon.ts` builds the SQL `LIMIT` clause from the query string directly. Be cautious when changing request handling there.
- `index.html` still has the default Vite title, so product polish tasks may need to update app metadata.
Comment on lines 60 to -61

@gemini-code-assist gemini-code-assist Bot Mar 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

この「Known pitfalls」から functions/api/routes/weapon.ts に関する警告を削除していますが、この警告はまだ有効だと思われます。
weapon.ts の L35 では、SQLクエリに文字列補間 (LIMIT ${parsedCount}) を使用しており、これはSQLインジェクションのリスクを伴う可能性があります。
parsedCount は数値であるか検証されていますが、セキュリティのベストプラクティスとして、このような記述は避けるべきです。
将来の開発者がこのリスクを認識できるよう、この警告を残しておくことを推奨します。

- `functions/api/routes/result.ts` keeps only the newest `MAX_HISTORY_ITEMS` entries. If you change that value, update the frontend cache trimming logic too.
- `src/app.tsx` disables several SWR revalidation paths and relies on mutation results for freshness. If you change that strategy, reevaluate KV read volume.
- `wrangler.toml` contains concrete binding IDs. Do not rotate or replace them casually.
Loading