fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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
AndresL230 merged 1 commit into from
Jul 28, 2026
Merged

fix(graph): dedupe subject-root synthesis per distinct course (#355) #447

Show file tree
Hide file tree
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
11 changes: 10 additions & 1 deletion backend/services/graph_service.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
if cid and cid in course_color_map:
n["course_color"] = course_color_map[cid]

# Build subject root hubs from enrolled courses
# Build subject root hubs from enrolled courses — one root per DISTINCT
# abstract course, not per enrollment. A user can hold two offerings of the
# same abstract course (e.g. re-taking it, or two sections); the graph stays
# keyed on the abstract course_id, so without this dedup the synthesized
# subject_root node (and its hub-spoke edges) would be duplicated once per
# extra enrollment (#355).
subject_nodes = []
subject_edges = []
seen_course_ids: set[str] = set()

for enrollment in enrolled_courses:
course_id = enrollment["course_id"]
if course_id in seen_course_ids:
continue
seen_course_ids.add(course_id)
course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
course_code = course.get("course_code", "")
course_name = course.get("course_name", "")
Expand Down
34 changes: 34 additions & 0 deletions backend/tests/test_graph_service.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
assert len(node["mastery_events"]) == 2
assert result["stats"]["avg_learning_velocity"] > 0

def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
"""#355: a user enrolled in TWO offerings of the same abstract course
(e.g. re-taking it, or two sections) must get exactly ONE subject-root
node — synthesis dedupes by abstract course_id, not per-enrollment."""
nodes = [
{"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
"mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
"times_studied": 2, "user_id": "u1"},
]
enrollments = [
_enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
_enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
]
factory = _mock_table({
"users": [{"streak_count": 0}],
"enrollments": enrollments,
"graph_nodes": nodes,
"graph_edges": [],
})
with patch("services.graph_service.table", side_effect=factory):
result = get_graph("u1")

node_ids = [n["id"] for n in result["nodes"]]
assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

roots = [n for n in result["nodes"] if n.get("is_subject_root")]
assert len(roots) == 1
assert roots[0]["id"] == "subject_root__c1"

# No surplus hub-spoke edges either: exactly one subject_edge per
# concept node under the course, not one per enrollment.
subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
assert len(subject_edges) == len(nodes)

def test_velocity_zero_when_no_events(self):
nodes = [
{"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down
51 changes: 28 additions & 23 deletions frontend/e2e/graph.spec.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
* collisions structurally irrelevant. Label TEXT is still asserted per node
* (it is part of the rendered data), just keyed by id.
*
* KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
* enrolled in two offerings of the SAME abstract course (subject-root
* synthesis iterates enrollments, not distinct courses). The rich seed
* intentionally has rich-user-active in both the F25 and S26 offerings of
* rich-course-cs101, so the exact-count test below trips on the duplicated
* `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
* PROBE355 nodes_total=18 nodes_unique=17
* FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
* user was enrolled in two offerings of the SAME abstract course
* (subject-root synthesis iterated enrollments, not distinct courses). The
* rich seed intentionally has rich-user-active in both the F25 and S26
* offerings of rich-course-cs101, so the exact-count test below is the
* regression coverage for it — it used to trip on the duplicated
* `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
* before the fix: PROBE355 nodes_total=18 nodes_unique=17
* dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
* edges_unique=20 dup_edges=5. That is this journey catching precisely the
* defect class it exists for — the correct assertion is kept and the test is
* marked fixme(#355) rather than relaxed. The companion tests assert
* everything #355 does not corrupt.
* edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
* roots by distinct abstract `course_id` (backend/services/graph_service.py),
* so this journey runs as a normal (non-fixme) exact-count test.
*/
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
const g = await graphExpectations();
expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

// Each subject-root hub renders exactly once per distinct enrolled course
// (the precise duplication #355 causes: these read 2 for CS101 today).
// (this is the precise regression coverage for #355: before the fix, this
// read 2 for CS101 for a user enrolled in two offerings of it).
for (const c of g.courses) {
await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

// Every DB concept node renders exactly once — in the SVG and in the a11y
// list — and shows its own concept name. Keyed by node id, so this holds
// even if two courses share a concept name. (Concept rows are what #355
// does NOT duplicate, so exact counts hold here; hub multiplicity lives in
// the fixme test above.)
// even if two courses share a concept name. (Concept rows were never
// affected by #355 — only the synthesized subject-root hub was — so exact
// counts hold here; the hub's own exact multiplicity is covered by the
// dedicated test above.)
for (const n of g.nodes) {
const item = byNodeId(items, n.id);
await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
}

// Every enrolled course's subject-root hub is present with its course
// label (≥1 — exact multiplicity is the #355-blocked assertion above).
// label (≥1 here; the dedicated test above asserts the exact multiplicity
// — this test stays a presence check so it doesn't duplicate that
// coverage).
for (const c of g.courses) {
const hub = byNodeId(items, rootId(c.course_id)).first();
await expect(hub).toBeVisible();
await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
}

// Edge floor: at least every DB edge + hub spoke is drawn (exact equality
// is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
// edge must still fail here).
// is asserted by the dedicated test above; this stays a floor check so a
// MISSING edge still fails here even independent of hub-count coverage).
expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

// Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(graph): dedupe subject-root synthesis per distinct course (#355)#447

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

AndresL230
merged 1 commit into

from

Jul 28, 2026
 Merged


fix(graph): dedupe subject-root synthesis per distinct course (#355)
#447
Show file tree
Hide file tree
 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


11 changes: 10 additions & 1 deletion
backend/services/graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,21 @@ def get_graph(user_id: str) -> dict:
 if cid and cid in course_color_map:
 n["course_color"] = course_color_map[cid]

 # Build subject root hubs from enrolled courses
 # Build subject root hubs from enrolled courses — one root per DISTINCT
 # abstract course, not per enrollment. A user can hold two offerings of the
 # same abstract course (e.g. re-taking it, or two sections); the graph stays
 # keyed on the abstract course_id, so without this dedup the synthesized
 # subject_root node (and its hub-spoke edges) would be duplicated once per
 # extra enrollment (#355).
 subject_nodes = []
 subject_edges = []
 seen_course_ids: set[str] = set()

 for enrollment in enrolled_courses:
 course_id = enrollment["course_id"]
 if course_id in seen_course_ids:
 continue
 seen_course_ids.add(course_id)
 course = enrollment.get("courses", {}) if isinstance(enrollment.get("courses"), dict) else {}
 course_code = course.get("course_code", "")
 course_name = course.get("course_name", "")
Expand Down

34 changes: 34 additions & 0 deletions
backend/tests/test_graph_service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,40 @@ def test_learning_velocity_computed_from_event_rows(self):
 assert len(node["mastery_events"]) == 2
 assert result["stats"]["avg_learning_velocity"] > 0

 def test_dedupes_subject_root_for_two_offerings_of_same_course(self):
 """#355: a user enrolled in TWO offerings of the same abstract course
 (e.g. re-taking it, or two sections) must get exactly ONE subject-root
 node — synthesis dedupes by abstract course_id, not per-enrollment."""
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
 "mastery_score": 0.5, "subject": "CS101", "course_id": "c1",
 "times_studied": 2, "user_id": "u1"},
 ]
 enrollments = [
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-1"),
 _enrollment_row("c1", "CS101", "Intro CS", offering_id="off-2"),
 ]
 factory = _mock_table({
 "users": [{"streak_count": 0}],
 "enrollments": enrollments,
 "graph_nodes": nodes,
 "graph_edges": [],
 })
 with patch("services.graph_service.table", side_effect=factory):
 result = get_graph("u1")

 node_ids = [n["id"] for n in result["nodes"]]
 assert len(node_ids) == len(set(node_ids)), f"duplicate node ids: {node_ids}"

 roots = [n for n in result["nodes"] if n.get("is_subject_root")]
 assert len(roots) == 1
 assert roots[0]["id"] == "subject_root__c1"

 # No surplus hub-spoke edges either: exactly one subject_edge per
 # concept node under the course, not one per enrollment.
 subject_edges = [e for e in result["edges"] if e["id"].startswith("subject_edge__")]
 assert len(subject_edges) == len(nodes)

 def test_velocity_zero_when_no_events(self):
 nodes = [
 {"id": "n1", "concept_name": "Loops", "mastery_tier": "learning",
Expand Down

51 changes: 28 additions & 23 deletions
frontend/e2e/graph.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -29,18 +29,18 @@
 * collisions structurally irrelevant. Label TEXT is still asserted per node
 * (it is part of the rendered data), just keyed by id.
 *
 * KNOWN BUG #355: /api/graph duplicates the subject-root hub when a user is
 * enrolled in two offerings of the SAME abstract course (subject-root
 * synthesis iterates enrollments, not distinct courses). The rich seed
 * intentionally has rich-user-active in both the F25 and S26 offerings of
 * rich-course-cs101, so the exact-count test below trips on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live:
 * PROBE355 nodes_total=18 nodes_unique=17
 * FIXED BUG #355: /api/graph used to duplicate the subject-root hub when a
 * user was enrolled in two offerings of the SAME abstract course
 * (subject-root synthesis iterated enrollments, not distinct courses). The
 * rich seed intentionally has rich-user-active in both the F25 and S26
 * offerings of rich-course-cs101, so the exact-count test below is the
 * regression coverage for it — it used to trip on the duplicated
 * `subject_root__rich-course-cs101` hub (+1 node, +5 spokes) — verified live
 * before the fix: PROBE355 nodes_total=18 nodes_unique=17
 * dup_node_ids=["subject_root__rich-course-cs101"] edges_total=25
 * edges_unique=20 dup_edges=5. That is this journey catching precisely the
 * defect class it exists for — the correct assertion is kept and the test is
 * marked fixme(#355) rather than relaxed. The companion tests assert
 * everything #355 does not corrupt.
 * edges_unique=20 dup_edges=5. `graph_service.get_graph` now dedupes subject
 * roots by distinct abstract `course_id` (backend/services/graph_service.py),
 * so this journey runs as a normal (non-fixme) exact-count test.
 */
import type { Locator, Page } from "@playwright/test";

Expand DownExpand Up@@ -174,11 +174,12 @@ function byNodeId(loc: Locator, id: string) {
 return loc.and(loc.page().locator(`[data-node-id=${JSON.stringify(id)}]`));
}

// Marked fixme, NOT relaxed: red today solely because of open bug #355 (see
// header). The duplicated CS subject root makes the UI render one node and
// five hub spokes more than a correct payload would. Un-fixme when #355
// lands — the assertions below are the acceptance test for it.
test.fixme("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
// Acceptance test for #355 (now fixed): before the fix, the duplicated CS
// subject root made the UI render one node and five hub spokes more than a
// correct payload would, so this ran as test.fixme. Promoted to a normal
// test now that graph_service.get_graph dedupes subject roots by distinct
// abstract course_id.
test("renders exactly one node per DB graph node plus one subject root per enrolled course, and one edge per DB edge plus one hub spoke per course node", async ({ page }) => {
 const g = await graphExpectations();
 expect(g.nodes.length).toBeGreaterThan(0); // journey guard: seeded graph present

Expand All@@ -195,7 +196,8 @@ test.fixme("renders exactly one node per DB graph node plus one subject root per
 await expect(svgEdges).toHaveCount(g.expectedEdgeCount);

 // Each subject-root hub renders exactly once per distinct enrolled course
 // (the precise duplication #355 causes: these read 2 for CS101 today).
 // (this is the precise regression coverage for #355: before the fix, this
 // read 2 for CS101 for a user enrolled in two offerings of it).
 for (const c of g.courses) {
 await expect(byNodeId(items, rootId(c.course_id))).toHaveCount(1);
 await expect(byNodeId(svgNodes, rootId(c.course_id))).toHaveCount(1);
Expand All@@ -211,9 +213,10 @@ test("renders every DB concept node exactly once, classified by its DB mastery s

 // Every DB concept node renders exactly once — in the SVG and in the a11y
 // list — and shows its own concept name. Keyed by node id, so this holds
 // even if two courses share a concept name. (Concept rows are what #355
 // does NOT duplicate, so exact counts hold here; hub multiplicity lives in
 // the fixme test above.)
 // even if two courses share a concept name. (Concept rows were never
 // affected by #355 — only the synthesized subject-root hub was — so exact
 // counts hold here; the hub's own exact multiplicity is covered by the
 // dedicated test above.)
 for (const n of g.nodes) {
 const item = byNodeId(items, n.id);
 await expect(item).toHaveCount(1);
Expand All@@ -222,16 +225,18 @@ test("renders every DB concept node exactly once, classified by its DB mastery s
 }

 // Every enrolled course's subject-root hub is present with its course
 // label (≥1 — exact multiplicity is the #355-blocked assertion above).
 // label (≥1 here; the dedicated test above asserts the exact multiplicity
 // — this test stays a presence check so it doesn't duplicate that
 // coverage).
 for (const c of g.courses) {
 const hub = byNodeId(items, rootId(c.course_id)).first();
 await expect(hub).toBeVisible();
 await expect(hub).toHaveText(rootLabel(c.course_code, c.course_name));
 }

 // Edge floor: at least every DB edge + hub spoke is drawn (exact equality
 // is #355-blocked — the duplicate hub adds surplus spokes, but a MISSING
 // edge must still fail here).
 // is asserted by the dedicated test above; this stays a floor check so a
 // MISSING edge still fails here even independent of hub-count coverage).
 expect(await svgEdges.count()).toBeGreaterThanOrEqual(g.expectedEdgeCount);

 // Mastery classification at the render layer: the 2D graph encodes the
Expand Down
Loading