diff --git a/frontend/src/components/screens/Dashboard.tsx b/frontend/src/components/screens/Dashboard.tsx index 8b474562..330ae0f2 100644 --- a/frontend/src/components/screens/Dashboard.tsx +++ b/frontend/src/components/screens/Dashboard.tsx @@ -24,7 +24,7 @@ import { type Assignment, } from "@/lib/api"; import type { GraphNode as ApiNode, GraphEdge as ApiEdge } from "@/lib/types"; -import { apiToGraphNode, type GraphNode, type GraphEdge } from "@/lib/data"; +import { apiToGraphNode, learnHrefForNode, type GraphNode, type GraphEdge } from "@/lib/data"; import { partitionCurrentAndArchive } from "@/lib/semesters"; import { IS_TEST_MODE, now } from "@/lib/testMode"; @@ -475,13 +475,7 @@ export function Dashboard() { width={size.w} height={size.h} highlightId={suggestNode?.id} - onNodeClick={(n) => { - const p = new URLSearchParams(); - p.set("topic", n.name); - p.set("mode", "socratic"); - if (n.course_id) p.set("course_id", n.course_id); - router.push(`/learn?${p.toString()}`); - }} + onNodeClick={(n) => router.push(learnHrefForNode(n))} /> {/* Courses key — sidebar layout only. Top-nav layout uses the full My Courses panel in the left column instead. */} @@ -1051,7 +1045,7 @@ export function Dashboard() { highlightId={suggestNode?.id} onNodeClick={(n) => { setFullscreen(false); - router.push(`/learn?topic=${encodeURIComponent(n.name)}&mode=socratic${n.course_id ? `&course_id=${encodeURIComponent(n.course_id)}` : ""}`); + router.push(learnHrefForNode(n)); }} /> diff --git a/frontend/src/components/screens/Tree.tsx b/frontend/src/components/screens/Tree.tsx index 3d985234..b5b8317f 100644 --- a/frontend/src/components/screens/Tree.tsx +++ b/frontend/src/components/screens/Tree.tsx @@ -14,7 +14,7 @@ import { getGraph, getCourses, getSessions, deleteGraphNode, type EnrolledCourse import { useToast } from "../ToastProvider"; import { useConfirm } from "@/lib/useConfirm"; import type { GraphNode as ApiNode, GraphEdge as ApiEdge } from "@/lib/types"; -import { apiToGraphNode, type GraphNode, type GraphEdge } from "@/lib/data"; +import { apiToGraphNode, learnHrefForNode, type GraphNode, type GraphEdge } from "@/lib/data"; type Tier = "all" | "mastered" | "learning" | "struggling" | "unexplored"; @@ -132,11 +132,13 @@ export function Tree() { return n?.id; }, [suggest, nodes]); - const onLearn = (n: GraphNode) => router.push( - `/learn?topic=${encodeURIComponent(n.name)}&mode=socratic${n.course_id ? `&course_id=${encodeURIComponent(n.course_id)}` : ""}`, - ); + const onLearn = (n: GraphNode) => router.push(learnHrefForNode(n)); + // Subject-root (course) nodes have no single quiz topic — open the picker + // rather than seeding the course name (mirrors the tutor fix, #319). The + // Quiz screen only reads `topic`/`concept`, so the old `course_id` param was + // dead and is dropped. const onQuiz = (n: GraphNode) => router.push( - `/quiz?topic=${encodeURIComponent(n.name)}${n.course_id ? `&course_id=${encodeURIComponent(n.course_id)}` : ""}`, + n.is_subject_root ? "/quiz" : `/quiz?topic=${encodeURIComponent(n.name)}`, ); const del = useConfirm(async () => { diff --git a/frontend/src/lib/data.test.ts b/frontend/src/lib/data.test.ts index 75959857..e6134af5 100644 --- a/frontend/src/lib/data.test.ts +++ b/frontend/src/lib/data.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it } from "vitest"; -import { apiToGraphNode, hashSeed, paletteFor } from "./data"; +import { apiToGraphNode, hashSeed, learnHrefForNode, paletteFor } from "./data"; import type { GraphNode as ApiNode } from "./types"; import type { EnrolledCourse } from "./api"; +import type { GraphNode } from "./data"; function makeApiNode(over: Partial = {}): ApiNode { return { @@ -107,6 +108,61 @@ describe("apiToGraphNode color resolution", () => { }); }); +function makeGraphNode(over: Partial = {}): GraphNode { + return { + id: "n1", + name: "Eigenvalues", + subject: "Linear Algebra", + color: "#7a874f", + is_subject_root: false, + mastery_tier: "learning", + mastery_score: 0.5, + course_id: "c1", + ...over, + }; +} + +describe("learnHrefForNode", () => { + it("seeds the topic from a concept node's name", () => { + const href = learnHrefForNode(makeGraphNode({ name: "Eigenvalues" })); + const params = new URLSearchParams(href.split("?")[1]); + expect(params.get("topic")).toBe("Eigenvalues"); + expect(params.get("mode")).toBe("socratic"); + expect(params.get("course")).toBe("c1"); + }); + + it("never sends a subject-root (course) node's name as the topic (#319)", () => { + const href = learnHrefForNode( + makeGraphNode({ name: "Linear Algebra", is_subject_root: true }), + ); + const params = new URLSearchParams(href.split("?")[1]); + expect(params.has("topic")).toBe(false); + // Still scoped to the course so the tutor opens filtered to it. + expect(params.get("course")).toBe("c1"); + expect(params.get("mode")).toBe("socratic"); + }); + + it("scopes the course via the `course` param the Learn screen reads", () => { + // Regression guard: the old handlers passed `course_id`, which Learn + // (searchParams.get("course")) silently ignored, dropping course scope. + const href = learnHrefForNode(makeGraphNode({ course_id: "c42" })); + const params = new URLSearchParams(href.split("?")[1]); + expect(params.get("course")).toBe("c42"); + expect(params.has("course_id")).toBe(false); + }); + + it("omits the course param when the node has no course_id", () => { + const href = learnHrefForNode(makeGraphNode({ course_id: "" })); + const params = new URLSearchParams(href.split("?")[1]); + expect(params.has("course")).toBe(false); + }); + + it("honors a non-default mode", () => { + const href = learnHrefForNode(makeGraphNode(), "flashcards"); + expect(new URLSearchParams(href.split("?")[1]).get("mode")).toBe("flashcards"); + }); +}); + describe("hashSeed", () => { it("is deterministic", () => { expect(hashSeed("hello")).toBe(hashSeed("hello")); diff --git a/frontend/src/lib/data.ts b/frontend/src/lib/data.ts index 6dec524b..2f3eeb7d 100644 --- a/frontend/src/lib/data.ts +++ b/frontend/src/lib/data.ts @@ -66,6 +66,26 @@ export type GraphEdge = { strength: number; }; +// Deep-link from a knowledge-graph node into the AI Tutor (`/learn`). +// +// Subject-root (course) nodes carry the *course* name, which must never be +// sent as the session topic: doing so pre-selected the course name as a topic +// on redirect (#319). For a course node we scope the tutor to the course and +// leave the topic empty so the learner picks a concept; concept nodes seed +// their own name as the topic. The course is passed as `course` — the param +// the Learn screen reads (`searchParams.get("course")`); the earlier handlers +// passed `course_id`, which Learn silently ignored, dropping course scope. +export function learnHrefForNode( + n: Pick, + mode = "socratic", +): string { + const p = new URLSearchParams(); + if (!n.is_subject_root) p.set("topic", n.name); + p.set("mode", mode); + if (n.course_id) p.set("course", n.course_id); + return `/learn?${p.toString()}`; +} + // Adapter from the backend `ApiNode` shape to the frontend `GraphNode` // shape consumed by `KnowledgeGraph`. Hoisted here from Tree/Learn/ // Dashboard so the three screens share a single source of truth.