Feat/counseling platform - #1015
Closed
wang97x wants to merge 9 commits into
Closed
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain, including stale student data, counseling integrity handling, and department-deletion behavior.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds counseling student records and role-based personal/team knowledge access across the FastAPI, PostgreSQL, and Vue platform.
Changes:
- Adds business roles, department-scoped student records, ownership, APIs, and migrations.
- Adds personal/team knowledge permissions and frontend workflows.
- Updates tests, documentation, branding, and WSL development support.
File summaries
| File | Reviewed change | Final review comments |
|---|---|---|
web/test/unit/teamKnowledge.test.js |
Team knowledge frontend coverage | — |
web/test/unit/personalKnowledge.test.js |
Personal knowledge frontend coverage | — |
web/src/views/StudentRecordsView.vue |
Student list, detail, assignment, and editing UI | critical (3 votes; L67): discard stale responses after student navigation. nit (1 vote; L128): add route, permission, and late-response tests. |
web/src/views/LoginView.vue |
Login branding | — |
web/src/views/ExtensionsView.vue |
Role-based extension visibility | — |
web/src/views/DataBaseView.vue |
Team/personal knowledge views | — |
web/src/views/DataBaseInfoView.vue |
Knowledge permissions and controls | — |
web/src/utils/shareConfig.js |
Share-scope classification | — |
web/src/utils/databaseCreateForm.js |
Personal knowledge request construction | — |
web/src/stores/user.js |
Business-role session state | moderate (1 vote; L35): map legacy admin to business_admin. moderate (1 vote; L189): use the complete mapping during refresh. |
web/src/stores/database.js |
Knowledge-base request and cache state | moderate (1 vote; L164): invalidate stale detail responses. moderate (1 vote; L688): prevent late query-parameter responses from mutating cleared state. |
web/src/router/index.js |
Student and knowledge route guards | — |
web/src/layouts/AppLayout.vue |
Student-record navigation | — |
web/src/components/knowledge/DatabaseCreateFlowModal.vue |
Personal/team creation flow | moderate (1 vote; L202): hide or disable sharing controls in personal mode. |
web/src/components/FileUploadModal.vue |
Role-based upload options | — |
web/src/components/EmbeddingModelSelector.vue |
Embedding model selection UI | — |
web/src/components/AgentChatComponent.vue |
Agent chat UI | — |
web/src/apis/knowledge_api.js |
Knowledge API client updates | — |
web/src/apis/counseling_api.js |
Counseling API client | — |
web/index.html |
Web entry and branding metadata | — |
web/eslint.config.js |
Frontend lint configuration | — |
docs/mechanisms/knowledge-base.md |
Knowledge-base mechanism documentation | — |
docs/develop-guides/roadmap.md |
Counseling roadmap status | nit (3 votes; L5): distinguish delivered scope from remaining work. |
docs/develop-guides/decisions/proposed/2026-09-12-counseling-minimal-release.md |
Counseling release proposal | — |
docs/develop-guides/decisions/implemented/2026-09-12-team-knowledge-entry.md |
Team knowledge decision record | — |
docs/develop-guides/decisions/implemented/2026-09-12-student-record-pages.md |
Student UI decision record | — |
docs/develop-guides/decisions/implemented/2026-09-12-personal-knowledge.md |
Personal knowledge decision record | — |
docs/develop-guides/decisions/implemented/2026-09-12-minimal-student-record.md |
Student-record decision record | — |
docs/develop-guides/decisions/implemented/2026-09-11-wsl-vscode-development.md |
WSL development decision record | — |
docs/develop-guides/contributing.md |
Contribution guidance | — |
docker-compose.wsl.yml |
WSL Compose support | — |
backend/test/unit/services/test_storage_migration.py |
Storage migration unit coverage | — |
backend/test/unit/services/test_oidc_service.py |
OIDC service unit coverage | — |
backend/test/unit/routers/test_knowledge_workspace_import.py |
Workspace import router coverage | — |
backend/test/unit/routers/test_knowledge_router_cleanup.py |
Knowledge router cleanup coverage | — |
backend/test/unit/permissions/test_resource_permission.py |
Resource permission unit coverage | — |
backend/test/unit/permissions/test_business_roles.py |
Business-role unit coverage | — |
backend/test/unit/knowledge/test_kb_utils.py |
Knowledge utility unit coverage | — |
backend/test/integration/services/test_schema_migration_version.py |
Schema migration integration coverage | — |
backend/test/integration/api/test_team_knowledge_api.py |
Team knowledge API coverage | — |
backend/test/integration/api/test_personal_knowledge_api.py |
Personal knowledge API coverage | — |
backend/test/integration/api/test_counseling_student_api.py |
Counseling student API coverage | nit (1 vote; L46): add a null-department business-admin rejection case with no created row. |
backend/test/integration/api/test_business_roles_api.py |
Business-role API coverage | — |
backend/server/utils/knowledge_permissions.py |
Knowledge permission dependencies | — |
backend/server/routers/workspace_router.py |
Workspace routes | — |
backend/server/routers/knowledge_router.py |
Knowledge authorization and routes | — |
backend/server/routers/knowledge_eval_router.py |
Knowledge evaluation routes | — |
backend/server/routers/counseling_router.py |
Counseling HTTP routes | — |
backend/server/routers/auth_router.py |
Business-role identity handling | — |
backend/server/routers/__init__.py |
Router registration | — |
backend/package/yuxi/storage/postgres/models_counseling.py |
Student-record schema | moderate (1 vote; L30): address department deletion behavior for referenced student records and add integration coverage. |
backend/package/yuxi/storage/postgres/models_business.py |
Business-role model fields | — |
backend/package/yuxi/storage/postgres/manager.py |
Schema version and migrations | — |
backend/package/yuxi/storage_migration.py |
Migration orchestration | — |
backend/package/yuxi/services/oidc_service.py |
OIDC service | — |
backend/package/yuxi/services/identity_admin_service.py |
Identity administration | — |
backend/package/yuxi/services/counseling.py |
Student-record business logic | moderate (2 votes; L38): reject missing departments before persistence. moderate (1 vote; L48): only map the department/code unique violation to 409; re-raise other integrity errors. |
backend/package/yuxi/repositories/counseling.py |
Department and counselor isolation | — |
backend/package/yuxi/permissions/resource_permission.py |
Resource authorization | — |
backend/package/yuxi/permissions/business_roles.py |
Business roles and capabilities | — |
backend/package/yuxi/permissions/__init__.py |
Permission exports | — |
backend/package/yuxi/knowledge/utils/kb_utils.py |
Knowledge utilities | — |
backend/package/yuxi/knowledge/manager.py |
Knowledge management | — |
backend/package/yuxi/config/static/info.template.yaml |
Static configuration template | — |
ARCHITECTURE.md |
Architecture and route documentation | — |
.vscode/tasks.json |
WSL development tasks | — |
.gitignore |
Repository ignore rules | — |
.gitattributes |
Repository attributes | — |
Review details
Suppressed comments (9)
backend/package/yuxi/services/counseling.py:50
- This catches every
IntegrityErroras a duplicate-code conflict, including foreign-key, NOT NULL, and check-constraint failures. Those failures are not 409 conflicts and the current mapping hides the real cause; only translate the department/code unique-constraint violation and re-raise other integrity errors for the route's generic handling.
except IntegrityError as exc:
await db.rollback()
raise FileExistsError("本部门学生编号已存在") from exc
backend/package/yuxi/storage/postgres/models_counseling.py:30
- 这个外键使用默认的
NO ACTION,而现有DELETE /api/departments/{id}会先迁移用户再直接删除部门;一旦部门已有学生档案,该删除会因counseling_students.department_id仍引用部门而抛出未处理的IntegrityError(500),破坏现有部门删除流程。请在部门删除流程中明确阻止并返回可理解的 4xx,或定义档案迁移/归档策略,并补充该场景的集成测试。
department_id = Column(Integer, ForeignKey("departments.id"), nullable=False)
backend/test/integration/api/test_counseling_student_api.py:51
- The integration coverage exercises cross-department and role denial, but not a
business_adminwhosedepartment_idisNULL. That path currently reaches the databaseNOT NULLconstraint and can be returned as a duplicate-code conflict; add a negative HTTP case asserting the intended rejection and that no row is created.
manager_id, manager = await actor(departments[0], "admin", '["business_admin"]')
owner_id, owner = await actor(departments[0], "user", '["counselor"]')
other_id, other = await actor(departments[0], "user", '["counselor"]')
tech_id, tech = await actor(departments[0], "superadmin", '["technical_admin"]')
_, foreign_manager = await actor(departments[1], "admin", '["business_admin"]')
_, no_role = await actor(departments[0], "user", "[]")
web/src/components/knowledge/DatabaseCreateFlowModal.vue:202
- 管理员也可以从“我的”分类打开该弹窗,但这里仍渲染可编辑的
ShareConfigForm;handleCreate在个人模式下无条件使用createPersonalShareConfig(),因此管理员填写的共享范围会被静默丢弃。个人模式应隐藏或禁用该表单,避免用户以为设置会生效。
web/src/stores/database.js:164 - The
kbId.value &&guard is false after leaving a detail route, because the account-switch watcher resetskbIdtonull. A response from the oldgetDatabaseInfo()request can therefore repopulatedatabaseafterresetFileBrowser()/navigation, leaving stale knowledge-base metadata in the shared store. Invalidate the request on selection reset (or use a generation token) before committing the response; the same identity rule must also cover the nested query-parameter load.
web/src/stores/database.js:688 - This duplicate conditional also accepts a late query-parameter response when
kbId.valuehas been cleared tonull. It then mutatesqueryParamsandmetafor no selected database, and those values can leak into the next detail view. Apply the same request invalidation used for database details before mutating shared state.
web/src/stores/user.js:190 getCurrentUser()repeats the incomplete legacy fallback, so a refresh can clear an existing legacy admin'sbusiness_admincapability and hide/studentswhile the backend continues to authorize it. Use the same full platform-role mapping as the login path rather than falling back to[]foradmin.
web/src/stores/user.js:35- This compatibility fallback only maps the legacy
userplatform role. The backend maps a legacyadmintobusiness_admin(backend/package/yuxi/permissions/business_roles.py:52-56), so an older login response withoutbusiness_rolesleaves the frontend with no business capabilities: the student-record navigation/guard and team-management controls disappear even though the backend still grants the role. Mirror the complete legacy mapping here (and in the refresh path below).
web/src/views/StudentRecordsView.vue:132 - The new student-record page has no web test covering its route-driven list/detail flow or permission-gated create/edit actions. The repository does maintain related frontend unit tests (for example
web/test/unit/database_store.test.jsandweb/test/unit/knowledge_detail_layout.test.js), so add a focused test for this page, including the late response after changingstudentId; otherwise the race and UI authorization regressions are not protected.
- Files reviewed: 68/69 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+67
to
+82
| async function loadDetail(id) { | ||
| loading.value = true | ||
| error.value = '' | ||
| detail.value = null | ||
| try { | ||
| detail.value = await counselingApi.getStudent(id) | ||
| editForm.value = { | ||
| background_summary: detail.value.background_summary, | ||
| status: detail.value.status | ||
| } | ||
| } catch (cause) { | ||
| error.value = cause.message || '加载档案详情失败' | ||
| } finally { | ||
| loading.value = false | ||
| } | ||
| } |
Comment on lines
+38
to
+40
| if BusinessCapability.ASSIGN_STUDENTS not in resolve_business_capabilities(actor): | ||
| raise PermissionError("需要学生分配权限") | ||
| repository = StudentRepository(db) |
|
|
||
| ## 知伴二次开发方向 | ||
|
|
||
| 知伴最小业务版继续补学生档案页面、团队与个人知识库及会话背景关联,真实模型联调安排在业务版验收之后。范围、取舍和分批验收见[实施提案](./decisions/proposed/2026-09-12-counseling-minimal-release.md)。当前已有基础品牌展示、可兼任的业务角色与学生档案存储和接口;辅导对话、档案页面与知识库业务流程仍待实现。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更说明
请说明本 PR 解决的问题、主要改动及影响范围。
feature/bug-fix/simplification/architecture/process/testing工程主张与 Owner
验证情况
每条验收主张单独一组,字段与决策记录的证据矩阵一致(主张、失败面、语义 Owner、证据、负向案例、结果)。结果词只使用
Passed、Inspected、Not run、Inferred;除Passed外不得写成测试通过。不适用的字段写「不适用」,不要留空。<验收主张>
涉及 Run、FIFO、SSE、沙盒、恢复或其他高风险 assembled path 时,请附 deterministic E2E 结果;真实 provider/browser 未执行时明确写
Not run和风险。简化 / 删除验收
仅
simplification必填:说明 keep/narrow/replace/remove 的取舍;列出 consumer、registration/export、配置/manifest、durable/wire/migration、测试、文档和依赖的负向搜索;写明“旧能力不存在”的证据与重新引入条件。其他类型填“不涉及”。独立语义 Review
请记录全新上下文 Reviewer 实际覆盖的需求、完整 diff 与测试范围、结论和未解决项;不粘贴推理流水账。Review 结论不能替代验证证据。
未验证范围与风险
请明确列出未执行的检查、无法复现的环境、外部 provider/浏览器/部署差异,以及对应风险。不要把未执行写成已通过。
事故反馈
若本变更修复达到门槛的高影响逃逸缺陷,请链接 postmortem 及已落地的 reproducer/guard/gate;未达到门槛请说明原因。
界面变更
如涉及界面或交互调整,请提供截图或录屏;不涉及请填写“不涉及”。
关联事项
如有关联 Issue,请填写
Closes #<issue-number>或相关链接;无关联事项请填写“无”。补充说明
请说明兼容性、配置、数据迁移或其他需要评审者关注的内容;无补充说明请填写“无”。