cleanup(phase8/task39): legacy ORM carve + old evaluation stack hard-delete + partial unique indexes#1670
Merged
Merged
Conversation
…odels Move the seven remaining non-eval ORM classes out of the legacy `aperag/db/models.py` aggregate into their natural per-domain `db/models.py`: * `Invitation` → `aperag.domains.identity.db.models` (carries `Role` enum reference at class-body time; this carve naturally dissolves the Layer C cross-domain `Role` import the old aggregate needed at module-top.) * `ConfigModel` / `Setting` / `UserQuota` → `aperag.domains.governance.db.models` * `ModelServiceProvider` (+ `ModelServiceProviderStatus` enum) / `PromptTemplate` → `aperag.domains.model_platform.db.models` * `ExportTask` (+ `ExportTaskStatus` enum) → `aperag.domains.knowledge_base.db.models` Rewrite consumer imports throughout `aperag/` and `config/` (`views/auth.py` / `views/quota.py` / `views/prompts.py` / `views/export.py` / `service/quota_service.py` / `service/export_service.py` / `config/export_tasks.py` plus the `aperag/db/repositories/*.py` layer). Per the canonical task call from 符炫炜 主架构师 (msg=8e76efa7), the ORM source module moves but the underlying tables stay; no schema change is needed for Part A itself — the partial-index changes ride in Part D's autogen. Ghost-check: phase8 task #39 canonical msg=8e76efa7 (carve 7 non-eval ORMs to domains). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Hard-delete the legacy evaluation framework that `aperag.domains.evaluation` (Phase 5 evaluation_v2) supersedes: * Service files removed: `aperag/service/evaluation_service.py` (514 LOC) + `aperag/service/question_set_service.py` (197 LOC). * Repository files removed: `aperag/db/repositories/evaluation.py` + `aperag/db/repositories/question_set.py`. Their mixins (`AsyncEvaluationRepositoryMixin` / `AsyncQuestionSetRepositoryMixin`) drop out of `aperag/db/ops.py::AsyncDatabaseOps`. * Celery tasks removed from `config/celery_tasks.py`: `initialize_evaluation_task` / `process_evaluation_batch_task` / `process_evaluation_item_task` / `reconcile_evaluations_task`, plus their dedicated `_new_async_engine` helper and the now-unused `asynccontextmanager` import. * Beat schedule entry `reconcile-evaluations` removed from `config/celery.py`. The four ORM tables (`evaluations` / `evaluation_items` / `questions` / `question_sets`) are dropped by the Part E alembic migration. Grep verified zero remaining callers in `aperag/` / `tests/` / `config/` before each removal. Ghost-check: phase8 task #39 canonical msg=8e76efa7 (hard-delete old evaluation stack, no migration). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ed util shim After Parts A + B carved or deleted every ORM class previously declared in `aperag/db/models.py`, the file collapses to its final post-modularization shape: * **Kept**: `Base` re-export from `aperag.db.base` (still used by `aperag/migration/env.py`, the graphindex test fixtures, and the `aperag/db/repositories/setting.py` consumer that previously did `from aperag.db import models as db_models`); `random_id` and `EnumColumn` shared helpers used by per-domain `db/models.py` modules; updated module docstring documenting the post-Phase-8 state and the natural Layer C dissolution. * **Removed**: 11 local ORM class definitions (7 carved in Part A, 4 deleted in Part B); the 3 enums that only those classes needed (`EvaluationStatus` / `EvaluationItemStatus` / `QuestionType` — `ModelServiceProviderStatus` rode along with the carve); the module-top `from aperag.domains.identity.db.models import OAuthAccount, Role, User` re-export shim that the old `Invitation.role` column type forced (Layer C 自然消除). Companion changes: * `aperag/db/repositories/setting.py` — switch from `from aperag.db import models as db_models` → `from aperag.domains.governance.db.models import Setting` now that `Setting` no longer lives on the legacy aggregate. * `aperag/migration/env.py` — drop the now-stale `import aperag.db.models` (no local mapper classes left to register against `Base.metadata` from that module) and update the surrounding comment. The G15 `test_phase4_consumer_domains_never_import_role_enum` boundary test continues to pass — it scans `aperag/domains/**`, which never had a `Role` import to begin with. The "exemption" the canonical referred to was the documented OK-by-design `Role` re-export at the top of `aperag/db/models.py`, which is now gone naturally. Ghost-check: phase8 task #39 canonical msg=8e76efa7 (Layer C 自然消除 after Invitation carve to identity). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… to partial index
Postgres treats every `NULL` as distinct, so a
`UniqueConstraint(<keys>, "gmt_deleted")` does not actually enforce
uniqueness over active rows (every active row has `gmt_deleted=NULL`,
all NULLs compare unequal, and duplicates slip through). Convert
each affected table to a partial unique index keyed by
`postgresql_where=text("gmt_deleted IS NULL")`:
* `aperag/domains/conversation/db/models.py::Chat` —
`uq_chat_bot_peer_deleted` (cols: `bot_id` / `peer_type` / `peer_id`
/ `gmt_deleted`) → partial unique index `uq_chat_bot_peer_active`
on `(bot_id, peer_type, peer_id)` where `gmt_deleted IS NULL`.
* `aperag/domains/marketplace/db/models.py::UserCollectionSubscription`
— `idx_user_marketplace_history_unique` (cols: `user_id` /
`collection_marketplace_id` / `gmt_deleted`) → partial unique index
`uq_user_subscription_marketplace_active` on `(user_id,
collection_marketplace_id)` where `gmt_deleted IS NULL`.
The `model_service_provider` table was carved into
`aperag.domains.model_platform.db.models` in Part A using the partial
index shape directly, so no separate change is needed there.
Schema migration is captured in the Part E alembic autogen.
Ghost-check: phase8 task #39 canonical msg=8e76efa7 + Weston cleanup
inventory v0 §5 (partial unique index for active rows).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…cy_orm_carve Single autogenerated alembic revision capturing the schema diff introduced by Parts A–D: * `drop_table` + matching index drops for the 4 legacy evaluation tables (`evaluations` / `evaluation_items` / `questions` / `question_sets`). The 7 carved ORMs (`Invitation` / `ConfigModel` / `UserQuota` / `Setting` / `ModelServiceProvider` / `PromptTemplate` / `ExportTask`) keep the same `__tablename__`, so autogen correctly produces no DDL for them — only the ORM source module moved. * `drop_constraint` + `create_index ... unique=True postgresql_where='gmt_deleted IS NULL'` for the three soft-delete tables: `chat` / `model_service_provider` / `user_collection_subscription` (Part D). * `downgrade()` recreates the dropped tables / constraints symmetrically. Verified locally: * `alembic upgrade head` from `6a8fc31427d9` succeeds, 4 evaluation tables drop, 3 partial unique indexes appear. * `alembic check` reports no new upgrade operations after upgrade. * Round-trip `downgrade -1` then `upgrade head` works. Ghost-check: phase8 task #39 canonical msg=8e76efa7 (one new alembic migration: drop 4 eval tables + partial unique index changes). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
earayu
force-pushed
the
bryce/phase8-task39-legacy-orm-carve
branch
from
3bec0a7 to
544ab7c
Compare
This was referenced Apr 25, 2026
earayu
added a commit
that referenced
this pull request
Apr 25, 2026
…ollow-up to #1670) (#1672) * cleanup(phase8): move export_collection_task to KB domain (Option β follow-up) Carve-finishing follow-up to #1670 (task #39): now that ExportTask ORM lives in aperag/domains/knowledge_base/db/models.py, the matching Celery task body can move to the KB domain too. - Move `export_collection_task` + `_build_manifest` from `config/export_tasks.py` to `aperag/domains/knowledge_base/tasks.py`, pinned with `name="config.celery_tasks.export_collection_task"` to match the convention of the other KB-domain tasks. - Update the only caller (`aperag.service.export_service`) to import from the new location. - Drop `config.export_tasks` from `config/celery.py` `include=` list. - Delete `config/export_tasks.py`. - Update KB models docstring to reflect the task body's new home. Pure code move + import path update — no behavior change. Gates green (21/21 boundary, 676 pass / 29 skip / 1 deselect / 0 fail unit suite, ruff clean). * style(phase8): apply ruff format to KB tasks.py CI lint-and-unit job runs `ruff format --check` (stricter than `ruff check`). My local pre-push gate only ran the latter, missing this. Pure mechanical reformat in the export_collection_task block — no semantic change. Architect canonical LGTM (msg=ca62c787) and Weston no-blocker CR (msg=e91a922c) both still apply since the move scope and task-name pin are unchanged. Note: indexing/tasks.py also flags ruff format check on origin/main itself (pre-existing drift carried in by an earlier merge); that file is outside this PR's move-only scope and not touched here.
This was referenced Apr 25, 2026
earayu
added a commit
that referenced
this pull request
Apr 25, 2026
…) (#1677) * refactor(phase8-g1): carve export v1 shim to knowledge_base domain (#47) Per PM scope (msg=7733c905) and Bryce inventory G1: complete the export carve trail started by #1670 (ExportTask ORM) and #1672 (export_collection_task). Pure carve / move; URL contract `/api/v1/export*` and `/api/v1/export-tasks*` unchanged. Changes: - aperag/views/export.py → aperag/domains/knowledge_base/api/export_routes.py (3 routes: create_export_task / get_export_task / download_export; switched User ORM dep → AuthenticatedUser Protocol, matching the canonical KB routes pattern) - aperag/service/export_service.py → aperag/domains/knowledge_base/service/export_service.py (git mv preserves history; replaced view_models.ExportTaskResponse references with bare ExportTaskResponse imported from KB schemas) - ExportTaskResponse schema migrated from aperag/schema/view_models.py to aperag/domains/knowledge_base/schemas.py (added to __all__); view_models.py keeps a backward-compat re-export following the same pattern used for knowledge_graph/retrieval schemas - aperag/app.py: import switched from aperag.views.export to aperag.domains.knowledge_base.api.export_routes; mount prefix remains /api/v1 (D1 hard-cut to /api/v2 deferred to later G3+ batch) Verification: - pytest tests/unit_test/test_modularization_boundaries.py -x → 21 passed (G1-G19 all green; new KB route does not import legacy aggregates) - pytest tests/unit_test/ → 686 passed, 29 skipped (matches phase8 baseline) - grep "from aperag.service.export_service|from aperag.views.export" returns 0 hits in aperag/, tests/, config/ - python -c "from aperag.app import app; ..." confirms 3 export endpoints remain at /api/v1/collections/{id}/export, /api/v1/export-tasks/{id}, /api/v1/export-tasks/{id}/download aperag/service/ retains exactly the 3 permanent seam files (quota_service / prompt_template_service / search_pipeline_service); no Layer A / Layer B violation. Ghost-check: none. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(phase8-g1): D7 v2 hard-cut for export endpoints (#47) Per @符炫炜 D7 canonical update (msg=30405f5b) + PM msg=3b0c2589: G1 includes URL prefix migration `/api/v1/export*` → `/api/v2/export*`, not just file carve. - aperag/app.py: export_router mount /api/v1 → /api/v2 - aperag/domains/knowledge_base/api/export_routes.py: docstring updated to reflect D7 v2 hard-cut - aperag/domains/knowledge_base/service/export_service.py: download_url template /api/v1/... → /api/v2/... - web/src/api-v2/schema.d.ts: 3 export route key strings v1 → v2 - web/src/features/collection/client-api.ts: 2 client call paths v1 → v2 - tests/unit_test/test_web_typed_api_contract.py: assertions and comment updated to expect v2 paths post-D7 Verification: - pytest tests/unit_test/test_modularization_boundaries.py tests/unit_test/test_web_typed_api_contract.py -x → 37 passed - python -c "from aperag.app import app; ..." → 3 export endpoints now mounted at /api/v2/{collections/{id}/export, export-tasks/{id}, export-tasks/{id}/download} - grep "/api/v1/export" aperag/ tests/ web/ config/ → 0 hits (a single unrelated historical comment about KG-eval export remains in knowledge_graph/api/routes.py:259, untouched) Ghost-check: none. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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.
Phase 8 task #39 — Legacy ORM carve + old evaluation hard-delete
Per 符炫炜 主架构师 canonical msg=8e76efa7 + PM msg=584a2546 lock + Weston inventory v0 §3+§5。
Scope (5 parts)
18c543e4a1328276aperag/db/models.pyto Base + utility only (377 → ~80 LOC, Layer C dissolved)bf0967087fc46d67a639b0e5515f(drop_table 4 + partial index changes)3bec0a7bPart A — 7 ORM carved
Invitationdomains/identity/db/models.pyConfigModeldomains/governance/db/models.pyUserQuotadomains/governance/db/models.pySettingdomains/governance/db/models.pyModelServiceProvider(+ModelServiceProviderStatus)domains/model_platform/db/models.pyPromptTemplatedomains/model_platform/db/models.pyExportTaskdomains/knowledge_base/db/models.pyPart B — Hard delete (per 符炫炜 msg=8e76efa7 canonical override)
aperag/db/models.py:Evaluation/EvaluationItem/Question/QuestionSet(+ enumsEvaluationStatus/EvaluationItemStatus/QuestionType)aperag/service/evaluation_service.py(514 LOC) +aperag/service/question_set_service.py(197 LOC) + 2 repositoriesconfig/celery_tasks.py: 4 old eval tasks removed (initialize_evaluation_task/process_evaluation_batch_task/process_evaluation_item_task/reconcile_evaluations_task)config/celery.py:reconcile-evaluationsbeat schedule removeddrop_tableforevaluations/evaluation_items/questions/question_sets— accepts data loss per @earayu2 msg=78fdb6fc / msg=f744412a hard-cut口径Part C — Layer C dissolved
aperag/db/models.pyreduced from 377 LOC → ~80 LOC. Removed:from aperag.domains.identity.db.models import OAuthAccount, Role, Userimport (Layer C exemption no longer needed — Invitation in identity domain)Kept:
Basere-export,random_id,EnumColumnutility class.Part D — Postgres NULL != NULL fix per Weston v0 §5
Soft-delete uniqueness was broken because
UniqueConstraint("name", "gmt_deleted")with nullable gmt_deleted lets multiple active rows share names. Converted 3 tables to partial unique indexesWHERE gmt_deleted IS NULL:chatmodel_service_provideruser_collection_subscriptionPart E — Alembic migration
Single revision
a639b0e5515f, post6a8fc31427d9 post_phase_7_initial. Verified locally:alembic upgrade head→ 0 error on running aperag-postgres containeralembic check→ no diff (终态稳定)alembic downgrade -1→alembic upgrade headround-trip clean§1 PERMANENT verification
quota_service.py+prompt_template_service.py— untouched ✅search_pipeline_service.py— untouched ✅app.py:101-145— untouched ✅Gates
rg "from aperag.db.models import (ConfigModel|UserQuota|Invitation|PromptTemplate|Setting|ExportTask|Evaluation|EvaluationItem|Question|QuestionSet|ModelServiceProvider)" aperag/ tests/ config/→ 0 hits ✅rg "from aperag.service.(evaluation_service|question_set_service)" aperag/ tests/ config/→ 0 hits ✅rg "from aperag.db.models import Role" aperag/ tests/→ 0 hits ✅pytest tests/unit_test/test_modularization_boundaries.py -x -q→ 20/20 pass (G1-G19 all green) ✅ruff check aperag/ tests/ config/→ clean ✅pytest tests/unit_test/ -x --ignore=objectstore -q→ 577 pass / 29 skip / 1 deselect (the 1 deselecttest_phase1_fe_complete_identity_auth_admin_audit_adapter_boundary missing listUserQuotasis pre-existing on baselinecd5f19d1, verified by stash+rerun, not introduced by this PR) ✅#36 / #39 parallel-merge bridge coordination (per 符炫炜 msg=6e1cf1e0)
#36 (dongdong) added a runtime bridge resolver in
aperag/domains/identity/api/auth_routes.pyto consumeInvitationwhile G1 forbids staticfrom aperag.db.models import Invitationfrom identity domain. 符炫炜 approved with mandatory cleanup in #39:domains/identity/db/models.pyauth_routes.pyruntime resolver with directfrom aperag.domains.identity.db.models import Invitation(within scope per canonical)Coord with other Phase 8 first-batch tasks
config/celery_tasks.pyregions (he migrates keep-celery 16 tasks, this PR removes 4 old eval task definitions).Net diff
32 files / +427 / -1740 / net ~1313 LOC deleted
Ghost-check
Ghost-check: none — §1 Layer A/B/D untouched; Layer C dissolved by canonical design; G1-G19 20/20 green; old eval stack hard-deleted per 符炫炜 msg=8e76efa7 explicit override.
🤖 Generated with Claude Code