From 913ccad5774df9a8e1f8259394658a524f8255d4 Mon Sep 17 00:00:00 2001 From: brother7 <7brother7@gmail.com> Date: Tue, 29 Sep 2026 12:03:06 +0800 Subject: [PATCH] =?UTF-8?q?fix(teacher):=20=E7=8B=AC=E7=AB=8B=E5=88=B7?= =?UTF-8?q?=E6=96=B0=E8=80=81=E5=B8=88=E7=9B=AE=E5=BD=95=E4=BB=A5=E7=BC=A9?= =?UTF-8?q?=E7=9F=AD=E9=85=8D=E7=BD=AE=E6=9B=B4=E6=96=B0=E5=BB=B6=E8=BF=9F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../20260929-teacher-publish-ml-e6b32168.md | 47 +++++++++++++ electron/api/routes/coding-teacher.ts | 9 ++- electron/coding-teacher/config-client.ts | 5 ++ electron/coding-teacher/service.ts | 5 ++ shared/coding-teacher.ts | 1 + src/lib/coding-teacher.ts | 1 + src/pages/Chat/use-teacher-companion.ts | 22 ++++-- tests/unit/teacher-catalog-refresh.test.tsx | 67 +++++++++++++++++++ tests/unit/teacher-catalog-revision.test.ts | 34 ++++++++++ 9 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 .project-docs/30-worklog/tasks/20260929-teacher-publish-ml-e6b32168.md create mode 100644 tests/unit/teacher-catalog-refresh.test.tsx create mode 100644 tests/unit/teacher-catalog-revision.test.ts diff --git a/.project-docs/30-worklog/tasks/20260929-teacher-publish-ml-e6b32168.md b/.project-docs/30-worklog/tasks/20260929-teacher-publish-ml-e6b32168.md new file mode 100644 index 00000000..eef414b0 --- /dev/null +++ b/.project-docs/30-worklog/tasks/20260929-teacher-publish-ml-e6b32168.md @@ -0,0 +1,47 @@ +# Task: Investigate teacher catalog refresh and pinned topic configuration + +## Identity + +- Task ID: 20260929-teacher-publish-ml-e6b32168 +- Mode: Feature +- Branch: codex/20260929-teacher-publish-ml-e6b32168-teacher-publish +- Worktree: D:\Datas\OthersProjects\.codex-worktrees\makelore\20260929-teacher-publish-ml-e6b32168 +- Base commit: 2767a1501759735684d1509a34c43eb2303784cf +- Owner: codex +- Status: Ready for Integration + +## Scope + +- Implemented independent teacher catalog refresh, small revision read through Renderer/Main/WS and normal passing regressions. Converted the earlier opt-in failing diagnostic into tests/unit/teacher-catalog-refresh.test.tsx; removed its diagnostic config. + +## Intent And Constraints + +- User approved implementation after choosing one explicit publish. The maintain-project-docs Concurrent and Planning Gates passed on resume at the recorded unchanged base/branch/worktree; original context and peer scope assessments remain applicable. All changes are isolated to this owned feature worktree. Preserve history, default/pause state, student billing and running-question versions. One fresh read-only Reviewer Agent is explicitly authorized for this pre-merge review only. No destructive data merge, production writes or canonical project-memory edits. + +## Outcome + +- Entry still loads the full catalog. Focus/visibility and five-second foreground checks read only the catalog revision, fetching full definitions when it changes. Reads are deduplicated and detached from chat, composing, running requests and legacy check-in conditions; hidden/unmounted scopes do not poll. The revision request uses the current account binding without an extra profile read and rejects responses after account changes. Current Main per-question authoritative catalog lookup and stored/running question snapshots remain intact. + +## Verification + +- Vitest focused six-file set (teacher-catalog-revision, teacher-catalog-refresh, teacher-companion, teacher-conversation, coding-teacher, coding-teacher-ui): 208 passed. Covered empty/never-started/running/completed states, five-second update, hidden pause, focus, unchanged catalogs, offline retry, unmount, account switch and existing conversation/history/project isolation. TypeScript noEmit, changed-file ESLint and build:vite passed. Existing shared node_modules was reused through a worktree-local junction; dependencies were not changed. Paired WS revision/registration tests passed and Yuxi actual editor verified. git diff --check passed. + +## Follow-ups + +- No commit/merge/client distribution. Deploy WS revision endpoint before installing this client, following the server/Yuxi/client order. Actual installed client and proxy latency remain unmeasured; five seconds is a foreground check interval, not a hard network SLA. Paired Yuxi full backend unit passed in the Python 3.13 runtime; the authorized fresh independent pre-commit review completed. + +## Promotion Candidates + +- Target: canonical teacher freshness/current-state and frontend flow documentation. Propose independent foreground revision polling with entry/focus refresh, preserving per-project continuous visible chat and per-question fixed execution. Evidence: previously failing actual-hook diagnosis now green plus 208 focused tests/build. Impact: newly delivered or updated teachers appear without waiting for chat lifecycle. Conflict: diagnostic-only status is superseded; no canonical files changed. Human confirmation: user-approved, promote only via Integration Gate. + +## Merge Preparation + +- User requested local main-branch integration and explicitly approved the three primary-checkout ownership handoffs, preserving seven foreign documents. Same-task feature check/start/status and Planning Gate passed at unchanged bases. Independent review and its bounded repair recheck passed; ready for the source commit. +- Yuxi full non-slow backend suite in existing WSL Python 3.13.13 passed: 2260 passed / 1 skipped. The previous Python 3.12 test-package shadowing limitation is superseded by this actual rerun; no unrelated code changed. + +## Independent Review Repair + +- The user-authorized fresh Reviewer found one actual compatibility defect: historic teacher ID coding-teacher was rejected by UUID-only publication schemas. WS native registration and legacy operations publication, plus Yuxi native publish, now accept string IDs of length 1..40 matching the database. +- Negative regressions reproduced HTTP 422 uuid_parsing in both repositories before the repair. WS actual HTTP now verifies choosing the legacy ID preserves default/pause, binding and historical versions; the complete relevant set including real PostgreSQL passed 33 cases. Yuxi actual HTTP/PostgreSQL UUID and legacy-ID cases passed both parameterized cases. Changed-file Ruff and Yuxi format checks passed. The same authorized Reviewer independently rechecked the bounded repair and confirmed the blocker closed, with no remaining actionable issue. + +- Final pre-commit review: fresh agent /root/teacher_merge_review inspected all three source diffs and untracked code/tests, found the legacy-ID defect, and confirmed its repair on 2026-09-29. Yuxi engineering verifier and all 62 verifier tests passed again after repair. Task documentation and whitespace checks complete the durable source handoff. diff --git a/electron/api/routes/coding-teacher.ts b/electron/api/routes/coding-teacher.ts index 2b7e8bbf..536fb59d 100644 --- a/electron/api/routes/coding-teacher.ts +++ b/electron/api/routes/coding-teacher.ts @@ -32,10 +32,11 @@ export async function handleCodingTeacherRoutes( const role = projectTopics?.[2] === 'friend' ? 'friend' : undefined; const config = url.pathname === '/api/coding/teacher/config' || url.pathname === '/api/coding/friend/config'; const catalog = url.pathname === '/api/coding/teacher/teachers'; + const catalogRevision = url.pathname === '/api/coding/teacher/teachers/revision'; const draft = url.pathname === '/api/coding/teacher-preview'; const pending = url.pathname === '/api/coding/teacher-preview/pending-link'; - if (!legacy && !conversation && !source && !preview && !projectTopics && !checkIn && !config && !catalog && !draft && !pending) return false; - if ((config || catalog || draft || pending) && req.method !== 'GET') { + if (!legacy && !conversation && !source && !preview && !projectTopics && !checkIn && !config && !catalog && !catalogRevision && !draft && !pending) return false; + if ((config || catalog || catalogRevision || draft || pending) && req.method !== 'GET') { sendJson(res, 405, { error: '不支持此操作。' }); return true; } @@ -82,6 +83,10 @@ export async function handleCodingTeacherRoutes( )); return true; } + if (catalogRevision) { + sendJson(res, 200, await service.catalogRevision()); + return true; + } if (catalog) { sendJson(res, 200, await service.catalog()); return true; diff --git a/electron/coding-teacher/config-client.ts b/electron/coding-teacher/config-client.ts index 0d735288..fec89f5b 100644 --- a/electron/coding-teacher/config-client.ts +++ b/electron/coding-teacher/config-client.ts @@ -73,6 +73,11 @@ export const teacherAvailability = (account: TeacherAccount) => teacherCloudRequest(account, '/api/coding-teacher/config'); export const teacherCatalog = (account: TeacherAccount) => teacherCloudRequest(account, '/api/coding-teacher/teachers'); +export async function teacherCatalogRevision(): Promise<{ revision: number }> { + const binding = getWorksSquareAccountBinding(); + if (!binding) throw new TeacherError(401, 'teacher_auth_required', '请先登录。'); + return teacherCloudRequest({ id: '', binding }, '/api/coding-teacher/teachers/revision'); +} export const teacherVersion = (account: TeacherAccount, version: number) => teacherCloudRequest<{ version: number; payload: TeacherDefinition }>( account, diff --git a/electron/coding-teacher/service.ts b/electron/coding-teacher/service.ts index 82d0d070..bd12a9b9 100644 --- a/electron/coding-teacher/service.ts +++ b/electron/coding-teacher/service.ts @@ -24,6 +24,7 @@ import { teacherVersion, teacherPreview, teacherCatalog, + teacherCatalogRevision, TeacherError, type TeacherAccount, } from './config-client'; @@ -103,6 +104,10 @@ export class CodingTeacherService { async catalog() { return (this.options.catalog ?? teacherCatalog)(await this.account()); } + + async catalogRevision() { + return teacherCatalogRevision(); + } async legacyHistory(projectId: string, selection?: { id: string; sourceId: string; friend: boolean }) { const account = await this.account(); const project = await this.options.projects.getProject(projectId); diff --git a/shared/coding-teacher.ts b/shared/coding-teacher.ts index a23dcf34..818fa7c9 100644 --- a/shared/coding-teacher.ts +++ b/shared/coding-teacher.ts @@ -40,6 +40,7 @@ export interface TeacherAvailability { revision: number; } export interface TeacherCatalog { + revision?: number; items: Array<{ teacher_id: string; version: number; definition: TeacherDefinition; is_default: boolean }>; } export interface TeacherReference { diff --git a/src/lib/coding-teacher.ts b/src/lib/coding-teacher.ts index 293f1bc7..aa9d15c4 100644 --- a/src/lib/coding-teacher.ts +++ b/src/lib/coding-teacher.ts @@ -37,6 +37,7 @@ export const teacherApi = { sendConversation: (projectId: string, agentId: string, input: TeacherSend) => hostApiFetch( teacherConversationsPath(projectId) + '/' + encodeURIComponent(agentId) + '/messages', { method: 'POST', body: JSON.stringify(input) }), catalog: () => hostApiFetch('/api/coding/teacher/teachers'), + catalogRevision: () => hostApiFetch<{ revision: number }>('/api/coding/teacher/teachers/revision'), checkIn: (projectId: string, input: TeacherCheckInInput) => hostApiFetch(`/api/coding/projects/${encodeURIComponent(projectId)}/teacher-check-in`, { method: 'POST', diff --git a/src/pages/Chat/use-teacher-companion.ts b/src/pages/Chat/use-teacher-companion.ts index ed5bf0ec..368527be 100644 --- a/src/pages/Chat/use-teacher-companion.ts +++ b/src/pages/Chat/use-teacher-companion.ts @@ -141,11 +141,14 @@ export function useTeacherCompanion(options: Options) { const base = projectId ? teacherTopicsPath(projectId) : ''; type LoadedCatalog = { enabled: boolean; selected?: TeacherCatalog['items'][number]; selectedId?: string }; let configFlight: Promise | null = null; + let catalogRevision: number | undefined; + let checkingCatalog = false; const loadConfig = (): Promise => { if (configFlight) return configFlight; configFlight = (async () => { const [config, catalog] = await Promise.all([teacherApi.config(), teacherApi.catalog()]); if (!alive || scopeRef.current !== scope) return { enabled: false }; + catalogRevision = catalog.revision; const selection = (current: CompanionState) => { const selectedId = current.selectedAgent?.teacher_id ?? current.topic?.definition.config_id ?? readLocal(storageKey + ':selected-agent', undefined); @@ -215,10 +218,7 @@ export function useTeacherCompanion(options: Options) { } // Delivered agents own one chat per project. Project observations // have their own Main scheduler; this legacy timer must not create topics. - if (currentState.selectedAgent || currentState.agents.length) { - try { await loadConfig(); } catch { /* Keep last known delivered contacts. */ } - return; - } + if (currentState.selectedAgent || currentState.agents.length) return; if (!projectId || current.sourceBusy || current.sourceArchived || !current.sourceId || (current.teacherOpen && current.teacherComposing)) return; if (Date.now() - lastAttempt < TEACHER_CHECK_IN_INTERVAL_MS) return; if (unreadInvitation(currentState)) return; @@ -252,13 +252,25 @@ export function useTeacherCompanion(options: Options) { } }; const timer = window.setInterval(() => { void check(); }, 15_000); - const onVisible = () => { void check(); }; + // Catalog freshness is independent of chat/check-in state and never dispatches a model. + const refreshCatalog = async () => { + if (!alive || checkingCatalog || document.visibilityState !== 'visible') return; + checkingCatalog = true; + try { + const current = await teacherApi.catalogRevision(); + if (alive && scopeRef.current === scope && current.revision !== catalogRevision) await loadConfig(); + } catch { /* Keep the last usable catalog; the next visible tick retries. */ } + finally { checkingCatalog = false; } + }; + const catalogTimer = window.setInterval(() => { void refreshCatalog(); }, 5_000); + const onVisible = () => { void refreshCatalog(); void check(); }; window.addEventListener('focus', onVisible); document.addEventListener('visibilitychange', onVisible); return () => { alive = false; if (configLoader.current === loadConfig) configLoader.current = null; window.clearInterval(timer); + window.clearInterval(catalogTimer); window.removeEventListener('focus', onVisible); document.removeEventListener('visibilitychange', onVisible); }; diff --git a/tests/unit/teacher-catalog-refresh.test.tsx b/tests/unit/teacher-catalog-refresh.test.tsx new file mode 100644 index 00000000..8072cbed --- /dev/null +++ b/tests/unit/teacher-catalog-refresh.test.tsx @@ -0,0 +1,67 @@ +// Actual hook against a changed server catalog, independently of chat activity. +import { act, cleanup, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import { useTeacherCompanion } from '@/pages/Chat/use-teacher-companion'; +import type { TeacherDefinition, TeacherTopic } from '../../shared/coding-teacher'; + +const api = vi.hoisted(() => ({ config: vi.fn(), catalog: vi.fn(), catalogRevision: vi.fn(), conversation: vi.fn(), list: vi.fn(), read: vi.fn(), events: vi.fn() })); +vi.mock('@/lib/coding-teacher', () => ({ teacherApi: api, teacherTopicsPath: (id: string) => id, teacherConversationsPath: (id: string) => id })); +vi.mock('@/stores/auth', () => ({ useAuthStore: (select: (state: unknown) => unknown) => select({ user: { userId: 'diagnostic-user' } }) })); +const definition: TeacherDefinition = { schema_version: 1, teacher_id: 'coding-teacher', config_id: 'agent-a', name: 'Old name', description: '', avatar_id: 'avatar-06', welcome_message: '', suggested_questions: [], system_prompt: '', skills: [], model: { model_id: 'model', reasoning_choice: { mode: 'default' } }, limits: { max_input_tokens: 8000, max_output_tokens: 1500 } }; +const original = { teacher_id: 'agent-a', version: 1, is_default: true, definition }; +const updated = { ...original, version: 2, definition: { ...definition, name: 'New name' } }; +beforeEach(() => { + vi.resetAllMocks(); vi.useFakeTimers(); localStorage.clear(); + Object.defineProperty(document, 'visibilityState', { configurable: true, value: 'visible' }); + api.config.mockResolvedValue({ enabled: true, definition }); + api.catalog.mockResolvedValue({ revision: 1, items: [original] }); + api.catalogRevision.mockResolvedValue({ revision: 1 }); + api.conversation.mockResolvedValue({ topic: null, before: null }); + api.list.mockResolvedValue({ items: [], lastSelectedTopicId: null }); + api.events.mockResolvedValue(Object.assign(new EventTarget(), { close: vi.fn() })); +}); +afterEach(() => { cleanup(); vi.useRealTimers(); }); + +it.each(['empty-catalog', 'never-started', 'running', 'completed'] as const)('refreshes changed catalog within five seconds with %s chat', async (state) => { + if (state === 'empty-catalog') api.catalog.mockResolvedValue({ revision: 1, items: [] }); + if (state === 'running' || state === 'completed') { + const topic: TeacherTopic = { schemaVersion: 1, revision: 1, id: 'topic', accountId: 'diagnostic-user', projectId: 'project', sourceConversationId: 'project', version: 1, definition, createdAt: 'now', updatedAt: 'now', conversation: { agentId: 'agent-a', segmentTurns: 1, discussions: {} }, requests: [{ id: 'request', text: 'Question', references: [], createdAt: 'now', sourceCursor: { workerGeneration: 1, seq: 1 }, sourceCapturedAt: 'now', includedSourceMessageIds: [], omittedMessages: 0, status: state, response: '' }] }; + api.conversation.mockResolvedValue({ topic, before: null }); + } + let view!: ReturnType, unknown>>; + await act(async () => { view = renderHook(() => useTeacherCompanion({ projectId: 'project', sourceId: 'source', sourceBusy: false, sourceArchived: false, teacherOpen: false, teacherComposing: false })); }); + expect(view.result.current.selectedAgent?.version).toBe(state === 'empty-catalog' ? undefined : 1); + api.catalog.mockResolvedValue({ revision: 2, items: [updated] }); + api.catalogRevision.mockResolvedValue({ revision: 2 }); + await act(async () => { await vi.advanceTimersByTimeAsync(5_000); }); + expect(view.result.current.selectedAgent?.version).toBe(2); + if (state === 'running') { + expect(view.result.current.topic?.requests[0].status).toBe('running'); + expect(view.result.current.topic?.version).toBe(1); + } +}); + +it('keeps unchanged catalogs, pauses while hidden, retries failures and refreshes immediately on focus', async () => { + let view!: ReturnType, unknown>>; + await act(async () => { + view = renderHook(() => useTeacherCompanion({ projectId: 'project', sourceId: 'source', sourceBusy: true, sourceArchived: false, teacherOpen: true, teacherComposing: true })); + }); + await act(async () => { await vi.advanceTimersByTimeAsync(10_000); }); + expect(api.catalog).toHaveBeenCalledTimes(1); + expect(api.catalogRevision).toHaveBeenCalledTimes(2); + Object.defineProperty(document, 'visibilityState', { configurable: true, value: 'hidden' }); + await act(async () => { await vi.advanceTimersByTimeAsync(10_000); }); + expect(api.catalogRevision).toHaveBeenCalledTimes(2); + Object.defineProperty(document, 'visibilityState', { configurable: true, value: 'visible' }); + api.catalogRevision.mockRejectedValueOnce(new Error('offline')); + await act(async () => { window.dispatchEvent(new Event('focus')); }); + expect(view.result.current.selectedAgent?.version).toBe(1); + api.catalogRevision.mockResolvedValue({ revision: 2 }); + api.catalog.mockResolvedValue({ revision: 2, items: [updated] }); + await act(async () => { window.dispatchEvent(new Event('focus')); }); + expect(view.result.current.selectedAgent?.version).toBe(2); + view.unmount(); + const calls = api.catalogRevision.mock.calls.length; + await act(async () => { await vi.advanceTimersByTimeAsync(5_000); window.dispatchEvent(new Event('focus')); }); + expect(api.catalogRevision).toHaveBeenCalledTimes(calls); +}); diff --git a/tests/unit/teacher-catalog-revision.test.ts b/tests/unit/teacher-catalog-revision.test.ts new file mode 100644 index 00000000..2aaacf1b --- /dev/null +++ b/tests/unit/teacher-catalog-revision.test.ts @@ -0,0 +1,34 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +const mock = vi.hoisted(() => ({ + binding: vi.fn(), current: vi.fn(), token: vi.fn(), fetch: vi.fn(), +})); +vi.mock('../../electron/services/works-square-session', () => ({ + getWorksSquareAccountBinding: mock.binding, + isCurrentWorksSquareAccountBinding: mock.current, + getValidWorksSquareAccessToken: mock.token, +})); +vi.mock('../../electron/utils/proxy-fetch', () => ({ proxyAwareFetch: mock.fetch })); +import { teacherCatalogRevision } from '../../electron/coding-teacher/config-client'; + +beforeEach(() => { + vi.resetAllMocks(); + mock.binding.mockReturnValue({ accountId: 'student', generation: 1 }); + mock.current.mockReturnValue(true); + mock.token.mockResolvedValue('student-token'); + mock.fetch.mockResolvedValue(new Response(JSON.stringify({ revision: 12 }))); +}); + +it('reads only the small directory revision with current account authentication', async () => { + expect(await teacherCatalogRevision()).toEqual({ revision: 12 }); + expect(mock.fetch).toHaveBeenCalledTimes(1); + expect(mock.fetch.mock.calls[0][0]).toMatch(/\/api\/coding-teacher\/teachers\/revision$/); + expect(mock.fetch.mock.calls[0][1].headers.Authorization).toBe('Bearer student-token'); +}); + +it('discards a response when the account changes while waiting', async () => { + mock.fetch.mockImplementation(async () => { + mock.current.mockReturnValue(false); + return new Response(JSON.stringify({ revision: 99 })); + }); + await expect(teacherCatalogRevision()).rejects.toMatchObject({ code: 'teacher_account_changed' }); +});