Skip to content

Commit 975be56

Browse files
committed
fix(#353,#354): conditionally set generatedAgentConfigs based on agentMode
#353: Only embed generatedAgentConfigs in the Stage when agentMode is 'generate'. For default mode, set agentIds instead. This prevents default agents from being incorrectly marked as isGenerated on the client-side registry. Also reset agentMode to 'default' in the catch block when LLM agent generation fails and falls back to defaults. #354: Remove redundant setSelectedAgentIds call from Path 1 (server hydration). Path 2 handles agent restoration uniformly for both IndexedDB and server-loaded classrooms. Includes unit tests covering all three scenarios (default, generate, and LLM fallback).
1 parent bb14de9 commit 975be56

3 files changed

Lines changed: 116 additions & 17 deletions

File tree

app/classroom/[id]/page.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,13 @@ export default function ClassroomDetailPage() {
5252
});
5353
log.info('Loaded from server-side storage:', classroomId);
5454

55-
// Hydrate server-generated agents into IndexedDB + registry
55+
// Hydrate server-generated agents into IndexedDB + registry.
56+
// Don't set selectedAgentIds here — the general agent
57+
// restoration logic below (Path 2) handles it uniformly.
5658
if (stage.generatedAgentConfigs?.length) {
5759
const { saveGeneratedAgents } = await import('@/lib/orchestration/registry/store');
58-
const { useSettingsStore } = await import('@/lib/store/settings');
59-
const agentIds = await saveGeneratedAgents(stage.id, stage.generatedAgentConfigs);
60-
useSettingsStore.getState().setSelectedAgentIds(agentIds);
61-
log.info('Hydrated server-generated agents:', agentIds);
60+
await saveGeneratedAgents(stage.id, stage.generatedAgentConfigs);
61+
log.info('Hydrated server-generated agents for stage:', stage.id);
6262
}
6363
}
6464
}

lib/server/classroom-generation.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ export async function generateClassroom(
229229

230230
// Resolve agents based on agentMode
231231
let agents: AgentInfo[];
232-
const agentMode = input.agentMode || 'default';
232+
let agentMode = input.agentMode || 'default';
233233
if (agentMode === 'generate') {
234234
log.info('Generating custom agent profiles via LLM...');
235235
try {
@@ -238,6 +238,7 @@ export async function generateClassroom(
238238
} catch (e) {
239239
log.warn('Agent profile generation failed, falling back to defaults:', e);
240240
agents = getDefaultAgents();
241+
agentMode = 'default';
241242
}
242243
} else {
243244
agents = getDefaultAgents();
@@ -328,17 +329,24 @@ export async function generateClassroom(
328329
style: 'interactive',
329330
createdAt: Date.now(),
330331
updatedAt: Date.now(),
331-
// Embed agent configs so API-generated classrooms can hydrate
332-
// the client-side agent registry without IndexedDB
333-
generatedAgentConfigs: agents.map((a, i) => ({
334-
id: a.id,
335-
name: a.name,
336-
role: a.role,
337-
persona: a.persona || '',
338-
avatar: AGENT_DEFAULT_AVATARS[i % AGENT_DEFAULT_AVATARS.length],
339-
color: AGENT_COLOR_PALETTE[i % AGENT_COLOR_PALETTE.length],
340-
priority: a.role === 'teacher' ? 10 : a.role === 'assistant' ? 7 : 5,
341-
})),
332+
// For LLM-generated agents, embed full configs so the client can
333+
// hydrate the agent registry without prior IndexedDB data.
334+
// For default agents, just record IDs — the client already has them.
335+
...(agentMode === 'generate'
336+
? {
337+
generatedAgentConfigs: agents.map((a, i) => ({
338+
id: a.id,
339+
name: a.name,
340+
role: a.role,
341+
persona: a.persona || '',
342+
avatar: AGENT_DEFAULT_AVATARS[i % AGENT_DEFAULT_AVATARS.length],
343+
color: AGENT_COLOR_PALETTE[i % AGENT_COLOR_PALETTE.length],
344+
priority: a.role === 'teacher' ? 10 : a.role === 'assistant' ? 7 : 5,
345+
})),
346+
}
347+
: {
348+
agentIds: agents.map((a) => a.id),
349+
}),
342350
};
343351

344352
const store = createInMemoryStore(stage);
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { describe, test, expect } from 'vitest';
2+
/**
3+
* Unit test for #353 fix: verify Stage object has correct agent fields
4+
* based on agentMode.
5+
*
6+
* This doesn't call any LLM — it directly tests the conditional logic
7+
* that was changed in classroom-generation.ts.
8+
*/
9+
10+
import { getDefaultAgents } from '@/lib/orchestration/registry/store';
11+
import { AGENT_COLOR_PALETTE, AGENT_DEFAULT_AVATARS } from '@/lib/constants/agent-defaults';
12+
13+
describe('#353: generatedAgentConfigs conditional on agentMode', () => {
14+
// Replicate the Stage construction logic from classroom-generation.ts L322-349
15+
function buildStageAgentFields(
16+
agentMode: 'default' | 'generate',
17+
agents: Array<{ id: string; name: string; role: string; persona?: string }>,
18+
) {
19+
return agentMode === 'generate'
20+
? {
21+
generatedAgentConfigs: agents.map((a, i) => ({
22+
id: a.id,
23+
name: a.name,
24+
role: a.role,
25+
persona: a.persona || '',
26+
avatar: AGENT_DEFAULT_AVATARS[i % AGENT_DEFAULT_AVATARS.length],
27+
color: AGENT_COLOR_PALETTE[i % AGENT_COLOR_PALETTE.length],
28+
priority: a.role === 'teacher' ? 10 : a.role === 'assistant' ? 7 : 5,
29+
})),
30+
}
31+
: {
32+
agentIds: agents.map((a) => a.id),
33+
};
34+
}
35+
36+
test('default mode should set agentIds, NOT generatedAgentConfigs', () => {
37+
const agents = getDefaultAgents();
38+
const fields = buildStageAgentFields('default', agents);
39+
40+
// Should have agentIds
41+
expect(fields).toHaveProperty('agentIds');
42+
expect((fields as any).agentIds).toEqual([
43+
'default-1', 'default-2', 'default-3',
44+
'default-4', 'default-5', 'default-6',
45+
]);
46+
47+
// Should NOT have generatedAgentConfigs
48+
expect(fields).not.toHaveProperty('generatedAgentConfigs');
49+
});
50+
51+
test('generate mode should set generatedAgentConfigs, NOT agentIds', () => {
52+
const agents = [
53+
{ id: 'gen-server-0', name: 'Prof. Li', role: 'teacher', persona: 'An expert' },
54+
{ id: 'gen-server-1', name: 'Assistant', role: 'assistant', persona: 'Helpful' },
55+
{ id: 'gen-server-2', name: 'Student', role: 'student', persona: 'Curious' },
56+
];
57+
const fields = buildStageAgentFields('generate', agents);
58+
59+
// Should have generatedAgentConfigs
60+
expect(fields).toHaveProperty('generatedAgentConfigs');
61+
expect((fields as any).generatedAgentConfigs).toHaveLength(3);
62+
expect((fields as any).generatedAgentConfigs[0].id).toBe('gen-server-0');
63+
64+
// Should NOT have agentIds
65+
expect(fields).not.toHaveProperty('agentIds');
66+
});
67+
68+
test('generate mode with LLM fallback should behave like default mode', () => {
69+
// Simulates: agentMode was 'generate', LLM failed, fell back to defaults
70+
// After our fix, agentMode is reset to 'default' in the catch block
71+
let agentMode: 'default' | 'generate' = 'generate';
72+
let agents;
73+
74+
try {
75+
throw new Error('Simulated LLM failure');
76+
} catch {
77+
agents = getDefaultAgents();
78+
agentMode = 'default'; // ← This is our fix
79+
}
80+
81+
const fields = buildStageAgentFields(agentMode, agents);
82+
83+
// Should behave exactly like default mode
84+
expect(fields).toHaveProperty('agentIds');
85+
expect(fields).not.toHaveProperty('generatedAgentConfigs');
86+
expect((fields as any).agentIds).toEqual([
87+
'default-1', 'default-2', 'default-3',
88+
'default-4', 'default-5', 'default-6',
89+
]);
90+
});
91+
});

0 commit comments

Comments
 (0)