Conversation
There was a problem hiding this comment.
Pull request overview
このPRは、AI問題生成APIのレスポンスを表示する結果画面を実装します。POST /api/v1/quest/generate からの生成結果を sessionStorage 経由で受け取り、タイトル、難易度、学習目標、ステップ一覧(アコーディオン形式)、参考リソースなどを表示するUIコンポーネント群を追加しています。既存の /exercises ページのタブUIおよびヘッダーデザインも、レトロゲーム風スタイルに統一されています。
Changes:
- AI生成演習の結果表示ページおよび関連コンポーネントを新規追加(結果ヘッダー、学習目標リスト、ステップカード、リソースリンク)
- Quest生成関連の型定義とAPIクライアントを追加
- 演習ページのヘッダーをダッシュボードと同じレトロゲーム風デザインに統一
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/features/exercise/types/index.ts |
Quest生成APIのリクエスト/レスポンス型、難易度ラベルを追加 |
frontend/src/features/exercise/api/generateQuest.ts |
Quest生成APIクライアント関数を新規作成 |
frontend/src/app/exercises/generate/result/page.tsx |
結果表示ページ(sessionStorageから読み込み、エラー/ローディング状態を処理) |
frontend/src/features/exercise/components/QuestResultHeader.tsx |
タイトル、難易度バッジ、所要時間を表示するヘッダー |
frontend/src/features/exercise/components/QuestObjectives.tsx |
学習目標を箇条書きで表示 |
frontend/src/features/exercise/components/QuestStepCard.tsx |
各ステップをアコーディオン形式で表示(コード例、チェックポイント含む) |
frontend/src/features/exercise/components/QuestStepList.tsx |
ステップ一覧の展開状態を管理 |
frontend/src/features/exercise/components/QuestResources.tsx |
参考URLリストを表示 |
frontend/src/features/exercise/components/AIGeneration.tsx |
PR説明では変更のみだが、実際はAPI統合が未実装 |
frontend/src/features/exercise/components/ExerciseMenu.tsx |
タブスタイルを更新、AIGenerationコンポーネントを統合 |
frontend/src/app/exercises/layout.tsx |
ヘッダーをレトロゲーム風デザインに更新 |
| const handleUrlSubmit = () => { | ||
| if (!url.trim()) return; | ||
| alert(`URLから問題を生成します: ${url}`); | ||
| }; |
There was a problem hiding this comment.
URL入力による問題生成機能が未実装です。handleUrlSubmit 関数は alert を表示するだけで、実際には generateQuest API を呼び出して、結果を sessionStorage に保存し、/exercises/generate/result へ遷移する必要があります。
現在の実装では、結果表示ページ (/exercises/generate/result/page.tsx) が sessionStorage.getItem("questGenerationResult") からデータを読み込もうとしていますが、このコンポーネントではデータを保存していないため、フローが完全に壊れています。
以下の実装が必要です:
generateQuestAPI を import- ローディング状態の管理
- API 呼び出しとエラーハンドリング
- 成功時に
sessionStorage.setItem("questGenerationResult", JSON.stringify(response))を実行 useRouterで/exercises/generate/resultへ遷移
| const handleDrop = (e: React.DragEvent<HTMLDivElement>) => { | ||
| e.preventDefault(); | ||
| setIsDragging(false); | ||
|
|
||
| const files = Array.from(e.dataTransfer.files); | ||
| const pdfFiles = files.filter(file => file.type === "application/pdf"); | ||
|
|
||
| if (pdfFiles.length > 0) { | ||
| alert(`${pdfFiles.length}個のPDFファイルをアップロードしました`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
PDF アップロード機能も未実装です。PR の説明では「PDF アップロード → API 連携は未実装(現状 alert stub のまま)。別 Issue 扱い。」と記載されていますが、URL 入力機能も同様に未実装であり、Issue #98 の完了条件「/exercises/generate でドキュメントを入力し「生成」ボタンを押すと POST /api/v1/quest/generate が呼ばれる」を満たしていません。
少なくとも URL 入力からの生成フローは、この PR で実装されるべきです。
| await navigator.clipboard.writeText(step.code_example); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); |
There was a problem hiding this comment.
navigator.clipboard.writeText の呼び出しにエラーハンドリングがありません。クリップボード API は HTTPS 環境でのみ動作し、失敗する可能性があります(ユーザーが権限を拒否した場合など)。
try-catch でエラーをキャッチし、エラー時にはユーザーに適切なフィードバックを提供する必要があります。
| await navigator.clipboard.writeText(step.code_example); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| if (!navigator || !navigator.clipboard || !navigator.clipboard.writeText) { | |
| alert("このブラウザではコードの自動コピー機能を利用できません。手動でコピーしてください。"); | |
| return; | |
| } | |
| try { | |
| await navigator.clipboard.writeText(step.code_example); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| } catch (error) { | |
| console.error("Failed to copy text to clipboard:", error); | |
| alert("コードのコピーに失敗しました。ブラウザの設定や権限を確認し、再度お試しください。"); | |
| } |
| const [checked, setChecked] = useState<boolean[]>( | ||
| () => Array(step.checkpoints.length).fill(false), | ||
| ); |
There was a problem hiding this comment.
チェックポイントの状態が、ステップが展開/折りたたみされるたびにリセットされます。checked state は QuestStepCard コンポーネント内で管理されていますが、親コンポーネント (QuestStepList) で expandedId が変更されると、コンポーネントが再マウントされるわけではないものの、ユーザーがステップを閉じてから再度開くと、チェック状態は保持されます。
しかし、useState の初期化関数 () => Array(step.checkpoints.length).fill(false) は初回マウント時のみ実行されるため、step.checkpoints の長さが変わった場合(別のクエストを表示する場合など)に配列の長さが一致しない可能性があります。
step.checkpoints.length が変わった場合に対応するため、useEffect で配列をリセットすることを検討してください。
| let detail = `ステータス: ${response.status}`; | ||
| try { | ||
| const json = JSON.parse(errorText); | ||
| if (json.detail) detail = json.detail; | ||
| } catch { | ||
| // ignore | ||
| } | ||
| throw new Error(`演習の生成に失敗しました。${detail}`); |
There was a problem hiding this comment.
API エラー時のエラーメッセージに、サーバーから返された生のステータスコードが含まれています。ユーザーフレンドリーではありません。
ステータスコード(400、401、403、500 など)に応じて、より具体的で分かりやすいエラーメッセージを提供することを検討してください。例:
- 400: 「入力内容に問題があります。」
- 401/403: 「認証に失敗しました。再度ログインしてください。」
- 500: 「サーバーエラーが発生しました。しばらく待ってから再試行してください。」
| let detail = `ステータス: ${response.status}`; | |
| try { | |
| const json = JSON.parse(errorText); | |
| if (json.detail) detail = json.detail; | |
| } catch { | |
| // ignore | |
| } | |
| throw new Error(`演習の生成に失敗しました。${detail}`); | |
| const status = response.status; | |
| // ステータスコードに応じたユーザーフレンドリーなメッセージ | |
| let userMessage = ""; | |
| switch (status) { | |
| case 400: | |
| userMessage = "入力内容に問題があります。"; | |
| break; | |
| case 401: | |
| case 403: | |
| userMessage = "認証に失敗しました。再度ログインしてください。"; | |
| break; | |
| case 500: | |
| userMessage = "サーバーエラーが発生しました。しばらく待ってから再試行してください。"; | |
| break; | |
| default: | |
| userMessage = "エラーが発生しました。しばらく待ってから再試行してください。"; | |
| break; | |
| } | |
| // サーバーからの詳細メッセージがあれば補足として付与 | |
| try { | |
| const json = JSON.parse(errorText); | |
| if (json.detail && typeof json.detail === "string") { | |
| userMessage += ` 詳細: ${json.detail}`; | |
| } | |
| } catch { | |
| // ignore JSON parse error | |
| } | |
| throw new Error(`演習の生成に失敗しました。${userMessage}`); |
| <button | ||
| type="button" | ||
| onClick={() => toggleCheckpoint(idx)} | ||
| className={`mt-0.5 h-5 w-5 shrink-0 border-2 border-[#14532D] flex items-center justify-center transition-colors ${ | ||
| checked[idx] ? "bg-[#4ADE80]" : "bg-white hover:bg-[#FDFEF0]" | ||
| }`} | ||
| aria-label={checked[idx] ? "完了済み" : "未完了"} | ||
| > | ||
| {checked[idx] && ( | ||
| <span className="text-[#14532D] font-bold text-xs">✓</span> | ||
| )} | ||
| </button> | ||
| <span | ||
| className={`text-sm font-medium ${ | ||
| checked[idx] ? "line-through text-[#14532D]/50" : "text-[#14532D]" | ||
| }`} | ||
| > | ||
| {point} | ||
| </span> | ||
| </li> |
There was a problem hiding this comment.
チェックボックス要素に視覚的なラベルがありません。スクリーンリーダーのユーザーは aria-label だけでは各チェックポイントの内容を理解できません。
<span> 要素(行 114-120)を <label> 要素で囲むか、チェックボックスとテキストの関連付けを明確にするために htmlFor と id を使用してください。
| <div | ||
| onClick={onToggle} | ||
| className="flex items-start gap-4 p-4 bg-white cursor-pointer hover:bg-[#F0F7F0] transition-colors" | ||
| > | ||
| <div className="flex-shrink-0 w-12 h-12 bg-[#4ADE80] border-2 border-[#14532D] flex items-center justify-center"> | ||
| <span className="text-2xl font-bold text-[#14532D]"> | ||
| {String(step.step_number).padStart(2, "0")} | ||
| </span> | ||
| </div> | ||
| <div className="flex-1 min-w-0"> | ||
| <h3 className="text-lg font-bold text-[#14532D] mb-1">{step.title}</h3> | ||
| <p className="text-sm text-[#14532D] line-clamp-2">{step.description}</p> | ||
| </div> | ||
| <div | ||
| className="flex-shrink-0 text-2xl text-[#14532D] transition-transform" | ||
| style={{ transform: isExpanded ? "rotate(90deg)" : "rotate(0deg)" }} | ||
| > | ||
| ▶ | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
アコーディオンのヘッダーには適切な ARIA 属性がありません。スクリーンリーダーのユーザーは、このボタンがアコーディオンであることや、展開/折りたたみ状態を理解できません。
以下の ARIA 属性を追加してください:
aria-expanded={isExpanded}- 展開状態を示すaria-controls="step-content-{step.step_number}"- 制御対象を示す- 展開コンテンツに対応する
id属性
| <h4 className="font-bold text-[#14532D] mb-3">📋 確認ポイント</h4> | ||
| <ul className="space-y-2"> | ||
| {step.checkpoints.map((point, idx) => ( | ||
| <li key={idx} className="flex items-start gap-3"> |
There was a problem hiding this comment.
チェックポイントのループでも配列のインデックスを key として使用しています。この場合、idx はチェック状態の配列のインデックスとしても使用されているため、順序が変わると状態の不整合が発生する可能性があります。
チェックポイントの内容自体を key として使用するか、バックエンドから一意な ID を返すようにすることを検討してください。
| <li key={idx} className="flex items-start gap-3"> | |
| <li key={point} className="flex items-start gap-3"> |
| const DIFFICULTY_COLOR: Record<string, string> = { | ||
| beginner: "bg-[#FCD34D]", | ||
| intermediate: "bg-[#FB923C]", | ||
| advanced: "bg-[#F87171]", | ||
| }; |
There was a problem hiding this comment.
色の定義がハードコードされています。このマッピングは QuestResultHeader.tsx にのみ存在し、他のコンポーネントでは使用されていません。
一貫性とメンテナンス性のため、この定義を型定義ファイル(features/exercise/types/index.ts)または専用の定数ファイルにエクスポートすることを検討してください。また、既存の QUEST_DIFFICULTY_LABELS の隣に配置することで、難易度関連の定数を一箇所にまとめることができます。
| "use client"; | ||
|
|
||
| import { useState } from "react"; | ||
|
|
||
| export function AIGeneration() { |
There was a problem hiding this comment.
PR の説明では「console.log 削除のみ(UI 構造は変更なし)」と記載されていますが、この diff では全ての行が新規追加として表示されています。
これは、PR #100 がまだマージされていないためと思われますが、実際には console.log の削除以外の変更(API 統合の欠如)が大きな問題です。PR の説明と実際の状態に齟齬があります。
このファイルで実際に何が変更されたのか(console.log の削除のみか、それとも全体が新規追加か)を明確にしてください。
- POST /api/v1/quest/generate のレスポンスを /exercises/generate/result で表示 - QuestResultHeader / QuestObjectives / QuestStepCard / QuestStepList / QuestResources コンポーネント追加 - generateQuest API クライアント追加(document_content のみ送信、user_rank は別 Issue で対応) - 遷移フロー: /exercises(問題生成タブ) API /exercises/generate/result - sessionStorage でページ間のレスポンスデータを受け渡し - AIGeneration.tsx: console.log 削除のみ(UI構造は変更なし)
6de34bf to
fc96d7c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
frontend/src/features/exercise/components/AIGeneration.tsx:121
- ボタンのスタイルが117行目で変更されていますが、変更理由が不明です。元々のコードには
flex items-center gap-2があり、絵文字とテキストの配置に使用されていましたが、この削除により、要素の配置が意図通りにならない可能性があります。
もし意図的な変更であれば問題ありませんが、偶発的な削除の場合は元に戻す必要があります。
<button
onClick={handleUrlSubmit}
className="px-8 py-3 bg-[#4ADE80] border-4 border-[#14532D] text-[#14532D] font-bold hover:bg-[#86EFAC] transition-all shadow-[4px_4px_0_#000] active:shadow-none active:translate-x-[4px] active:translate-y-[4px]"
>
<span className="text-lg">生成</span>
<span className="text-xl">🚀</span>
</button>
| } | ||
| try { | ||
| setQuest(JSON.parse(raw) as QuestGenerationResponse); | ||
| } catch { |
There was a problem hiding this comment.
空のtry-catchブロックで、sessionStorageのJSONパースエラーを無視しています。デバッグ時に問題の原因を特定しやすくするため、エラーログを出力するべきです。
例:
} catch (e) {
console.error("Failed to parse questGenerationResult from sessionStorage:", e);
setError("データの読み込みに失敗しました。演習生成ページからやり直してください。");
}
| } catch { | |
| } catch (e) { | |
| console.error("Failed to parse questGenerationResult from sessionStorage:", e); |
| async function copyCode() { | ||
| if (!step.code_example) return; | ||
| await navigator.clipboard.writeText(step.code_example); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| } |
There was a problem hiding this comment.
navigator.clipboard.writeTextはセキュアコンテキスト(HTTPS)でのみ動作します。また、ユーザーがクリップボード権限を拒否した場合、エラーが発生します。エラーハンドリングを追加することを推奨します。
例:
async function copyCode() {
if (!step.code_example) return;
try {
await navigator.clipboard.writeText(step.code_example);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch (error) {
console.error("Failed to copy code:", error);
// フォールバック処理やユーザーへの通知
}
}- フロントエンド: AIGeneration.tsxでAPI連携と画面遷移を実装 - generateQuest APIを呼び出してsessionStorageに結果を保存 - URL入力からテキストエリア(10〜10,000文字)に変更 - ローディング状態とエラーハンドリングを追加 - バックエンド: QuestGenerationRequestのuser_rankをオプショナルに変更 - デフォルト値0(初心者)を設定し、フロントエンドから省略可能に - バックエンド: LLMレスポンスのMarkdownコードブロック(```json)除去処理を追加 - OpenAIがJSONをMarkdownで囲んで返す問題を解決 - フロントエンド: 422エラーの詳細表示を改善 - FastAPIのバリデーションエラーをフィールド名付きで表示 - フロントエンド: react-markdownとremark-gfmを依存関係に追加 - Lint修正: any型を具体的な型に変更、Tailwindクラス最適化
| print(f"❌ JSON parse error: {e}") | ||
| print(f"📝 LLM Response (first 500 chars): {response[:500]}") | ||
| print(f"📝 LLM Response (last 500 chars): {response[-500:]}") | ||
| print("⚠️ Returning fallback response.") |
There was a problem hiding this comment.
Quest生成失敗時に LLM のレスポンス(先頭/末尾500文字)をそのまま print しており、学習ドキュメント由来の機微情報がログに残る可能性があります。backend では他サービスが logging.getLogger(name) を使っているので(例: backend/app/services/rank_service.py)、本件も logger に寄せてレベル制御(debug のみ出力)し、必要なら内容をマスク/短縮した上で出力してください。
| # 必須フィールドの存在確認 | ||
| required_fields = ["title", "difficulty", "steps"] | ||
| if not all(k in result for k in required_fields): | ||
| raise ValueError("Missing required fields in LLM response") | ||
| return result |
There was a problem hiding this comment.
必須フィールドチェックが title/difficulty/steps のみのため、LLMが estimated_time_minutes や learning_objectives を欠いたJSONを返すと、この関数は成功扱いで返却し、呼び出し側の QuestGenerationResponse 検証で 500 になり得ます。ここで QuestGenerationResponse の必須項目(estimated_time_minutes, learning_objectives, resources など)も検証するか、欠損時はデフォルト値補完/フォールバックに落とすようにして、500 を避けてください。
| const DIFFICULTY_COLOR: Record<string, string> = { | ||
| beginner: "bg-[#FCD34D]", | ||
| intermediate: "bg-[#FB923C]", | ||
| advanced: "bg-[#F87171]", | ||
| }; |
There was a problem hiding this comment.
DIFFICULTY_COLOR が Record<string, string> になっており、quest.difficulty の取り得る値との対応が型で保証されません。QuestDifficulty(または Props の difficulty 型)に合わせて Record<QuestDifficulty, string> のように型付けすると、キー抜け/タイポをコンパイル時に検出できます。
| const loadQuest = () => { | ||
| const raw = sessionStorage.getItem("questGenerationResult"); | ||
| if (!raw) { | ||
| setError( | ||
| "生成結果が見つかりません。演習生成ページからやり直してください。", | ||
| ); | ||
| return; | ||
| } | ||
| try { | ||
| setQuest(JSON.parse(raw) as QuestGenerationResponse); |
There was a problem hiding this comment.
sessionStorage のデータが無い/壊れている場合にエラー表示だけしてキーを残したままなので、ブラウザバック等で再度このページに来ると同じエラーが再現し続けます。raw が無い/JSON.parse に失敗したケースでは questGenerationResult を removeItem してからエラーメッセージを出す方が復帰しやすいです。
| import { useRouter } from "next/navigation"; | ||
| import { generateQuest } from "../api/generateQuest"; | ||
|
|
||
| export function AIGeneration() { | ||
| const [url, setUrl] = useState(""); | ||
| const router = useRouter(); | ||
| const [documentContent, setDocumentContent] = useState(""); | ||
| const [isDragging, setIsDragging] = useState(false); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); |
There was a problem hiding this comment.
PR説明では「console.log削除のみ」とありますが、このファイルではAPI呼び出し・入力UIの刷新・sessionStorageへの保存・結果ページ遷移など挙動が大きく変更されています。レビューポイントが変わるため、PR説明を実際の変更内容に合わせて更新してください。
- フロントエンド: AIGeneration.tsxでAPI連携と画面遷移を実装 - generateQuest APIを呼び出してsessionStorageに結果を保存 - URL入力からテキストエリア(10〜10,000文字)に変更 - ローディング状態とエラーハンドリングを追加 - バックエンド: QuestGenerationRequestのuser_rankをオプショナルに変更 - デフォルト値0(初心者)を設定し、フロントエンドから省略可能に - バックエンド: LLMレスポンスのMarkdownコードブロック(```json)除去処理を追加 - OpenAIがJSONをMarkdownで囲んで返す問題を解決 - フロントエンド: 422エラーの詳細表示を改善 - FastAPIのバリデーションエラーをフィールド名付きで表示 - Lint修正: useEffect内setState、未使用変数、any型を修正 - テスト修正: JWT署名検証エラーハンドリングを改善
94d6777 to
af5f6c7
Compare
Issue: #98
実装の概要
POST /api/v1/quest/generateのレスポンスを受け取り、/exercises/generate/resultで表示する画面を実装。遷移フロー:
/exercises(問題生成タブ) URL入力 API呼び出し 結果表示ページ追加ファイル
app/exercises/generate/result/page.tsxfeatures/exercise/api/generateQuest.tsfeatures/exercise/components/QuestResultHeader.tsxfeatures/exercise/components/QuestObjectives.tsxfeatures/exercise/components/QuestStepCard.tsxfeatures/exercise/components/QuestStepList.tsxfeatures/exercise/components/QuestResources.tsx変更ファイル
features/exercise/types/index.ts: Quest 生成関連の型定義を追加features/exercise/components/AIGeneration.tsx:console.log削除のみ(UI 構造は変更なし)🔧 技術的な意思決定とトレードオフ (最重要)
採用したアプローチ
sessionStorageによる API レスポンスのページ間受け渡し却下したアプローチ(代替案)
手法: URL クエリパラメータでデータ受け渡し
却下理由: レスポンスが大きく URL サイズ制限に抵触する可能性があるため
手法: グローバルな状態管理(Zustand 等)
却下理由: 既存コードに状態管理ライブラリが導入されておらず、このスコープで追加は過剰
user_rankの扱いフロントからは
document_contentのみ送信。user_rank連携は別 Issue で対応予定。🧪 テスト戦略と範囲
追加したテストケース
静的解析
npx tsc --noEmit: pass ✅セキュリティに関する自己評価
NEXT_PUBLIC_API_URL環境変数から取得)レビュワー(人間)への申し送り事項
AIGeneration.tsxはconsole.log削除のみで UI 変更なし(AIから問題生成ページの実装 #97 の成果物を破壊していない)/exercises/web/web-html-basicsの既存 UI に合わせたレトロゲーム風Closes Frontend: AI生成ハンズオン演習の結果表示画面の実装 #98