Files
model-platform/CLAUDE.md
T
d196b237a7 refactor(schedule): move orchestrator to application/ + review fixes (stage 8)
Follow-up to the layered refactor (review-driven):

- Move scheduling/orchestrator.py -> application/orchestrator.py
  (orchestrator is application-level coordination, not a cron-trigger
  primitive; matches the intended target tree)
- Migrate orchestrator re-exports from scheduling/__init__.py to
  application/__init__.py; scheduling/ now exposes only CronScheduler
- Rewrite imports + 5 mock.patch string targets in test_janitor.py and
  the orchestrator import in test_layering.py
- Update docstring refs in application/service.py + execution/worker.py
- Add 4 runner smoke tests (test_layering.py): _limited_log under-limit /
  empty-sentinel / above-MAX_LOG_BYTES truncation; execute_artifact
  rejects unsupported script_type with ValueError
- infrastructure/__init__.py re-exports SchedulerStorageClient so
  ``from schedule.infrastructure import SchedulerStorageClient`` is a
  stable top-level surface
- CLAUDE.md engineering note: extend the commit trail to 7476c27 and
  note the stage-8 orchestrator placement

Zero behavior change; schedule/pyproject.toml untouched. 29 tests green.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-09-02 10:10:41 +08:00

12 KiB
Raw Blame History

Repository Guide

Architecture

  • frontend — React Router v8 SPA; production bundle built in nginx/Dockerfile.
  • backend — public FastAPI API + internal S3 storage API in one process.
  • runtime — Jupyter lifecycle, MySQL edit leases, short-lived in-memory access tickets.
  • schedule — APScheduler, MySQL JobStore, MySQL Outbox polling and DAG execution.
  • common — SQLAlchemy models, database/session helpers, IDs, object-store helpers.
  • migrations — Alembic schema + seed migrations.
  • nginx — static frontend, /api/ proxy, authenticated /jupyter/ proxy.

Redis and the former separate Storage API container are intentionally removed.

Commands

From repo root:

uv sync --all-packages
uv run python -m compileall common/src backend/src runtime/src schedule/src
uv run --package backend alembic upgrade head
uv run --package backend pytest backend/tests -q

cp .env.example .env docker compose config docker compose up -d --build


Frontend:

```bash
cd frontend
pnpm install
pnpm dev
pnpm typecheck   # runs `react-router typegen && tsc`
pnpm build

Service rules

  • Browser traffic enters through Gateway only; frontend uses same-origin /api/v1/....
  • Backend writes schedule_runs and outbox_events, then besteffort HTTP-dispatches to Schedule Executor. The executor always polls pending MySQL Outbox rows, so dispatch failure does not lose a task.
  • Cron jobs persisted in MySQL apscheduler_jobs.
  • Runtime must stay single-replica while file leases and Jupyter tickets use the simplified implementation.
  • Never expose the internal Jupyter token to the browser.
  • Never delete Docker volumes when preserving MySQL or storage data.

Engineering notes

Hard-won lessons. Read the relevant bullet before touching the named area.

Python runtime — always go through uv run

  • Repo is a uv workspace; common / backend / runtime / schedule / migrations share one .venv. Bare python / pytest / alembic resolves to system Python and all from backend.X import ... / from common.X import ... fail with ModuleNotFoundError, or worse: an out-of-date venv silently runs stale code.
  • Always prefix with uv run [--package <pkg>] <cmd>:
    • uv run --package backend pytest backend/tests -q
    • uv run --package backend alembic upgrade head / downgrade -1
    • uv run python -m compileall common/src backend/src runtime/src schedule/src
    • uv run python -c "from backend.foo import bar" for one-shot inspection
  • Migration files use plain op.drop_index / op.create_index — MySQL 8.0 does not support IF EXISTS / IF NOT EXISTS on DROP INDEX / CREATE INDEX, even though Alembic exposes the flag.

Platform auth / soft-delete (/api/v1/platform/employees)

  • Reuse system_admin_context + _*_admins helpers in backend/src/backend/platform.py. Self-protection and the last-admin guard for Users.platform_role_id mirror the workspace pattern. _count_active_system_admins(session, exclude_user_id=...) is already there; do not reinvent the count in the handler.
  • PATCH guard order is load-bearing. Always check is_current_system_admin last-admin first, then leaves_admin_pool, then self-demotion. Reversing the order lets test mocks bypass the count helper.
  • 404 vs 409 on already-soft-deleted: delete_platform_employee returns a single 404 "用户不存在" for both user is None and user.is_deleted != 0. This intentionally differs from DELETE /workspaces/{id} which returns 409 for already-disabled. Document both in API.md.
  • PATCH cannot null-out platform_role_id. PlatformEmployeeUpdate.role_code: Literal["admin","developer"] (not Optional). Add a separate endpoint if clearing is needed — never loosen the Literal.
  • DELETE cascade covers WorkspaceMembers only — writes is_deleted=1, deleted_at=now on WorkspaceMembers rows for the user. Does not touch Workspaces.

Backend test mocks (SQLAlchemy 2.0 / pytest-asyncio)

  • compile(literal_binds=True) uppercases keywords. "from roles" in text misses the table reference; match text.lower() or test for "roles.role_id" / "roles.role_code" directly.
  • Depends-style helpers are awaited. _load_role_by_code and _count_active_system_admins need async def mocks — sync lambda raises TypeError: object int can't be used in 'await' expression.
  • Mock response ordering matters for re-reads. update_platform_employee reads current_role before write then response_role after. A scalar mock returning a fixed value will return the pre-write role in the response — track call order or look up by user.platform_role_id post-write.
  • Mock users must declare every attribute the handler writes. Users.deleted_at is not in column defaults; SimpleNamespace(user_id=...) raises AttributeError on target.deleted_at = now. Set user.deleted_at = None explicitly.

Schedule service layering (domain / scheduling / application / execution / infrastructure)

Lessons from the zero-behavior-change refactor that split the flat 11-file schedule/src/schedule/ into five subpackages (commits 31186947476c27, with a follow-up git mv in stage 8 placing the orchestrator under application/). Public class names (SchedulerService, CronScheduler, DispatchOrchestrator, NodeExecutor, SchedulerStorageClient) and schedule/pyproject.toml are unchanged.

  • python -m schedule.notebook_runner is a stable external contract. The worker launches the notebook subprocess with sys.executable, "-m", "schedule.notebook_runner". That -m string must never change — so schedule/notebook_runner.py survives as a 6-line shim re-exporting main from schedule.execution.runners.notebook. Don't "clean up" the shim.
  • Mock patch() string targets and in-function lazy imports are invisible to import-line greps. test_janitor.py patched "schedule.orchestrator.session_scope" and test_worker.py patched "schedule.service.build_object_store"; worker.py also had a from schedule.service import ... inside a function body. When files move, these silently no-op against the old path — and hard-crash once the old file is deleted. After any move, grep the whole tree for the old module path (including tests) and rewrite every hit, not just top-level imports.
  • A new package dir shadows a same-named flat module. Creating schedule/execution/ makes the old schedule/execution.py silently dead code (the package wins import resolution), so move-then-delete, don't just copy. git usually detects these as renames, which keeps the diff reviewable.
  • Docstring references survive file deletion. After removing flat files, :class:\schedule.worker.NodeExecutor``-style text can linger in docstrings and render as broken links. Grep for the old module name one more time at cleanup and rewrite comment-only refs too.

Frontend state + routing (zustand + React Router v8)

Lessons from splitting frontend/app/features/platform/ModelPlatformApp.tsx (1057 → 121 lines) into zustand stores + nested routes.

  • Barrel-file CSS imports vanish on refactor. features/admin/AdminPages.tsx was a barrel that side-effect-imported admin.css + dashboard.css. When route files started importing DashboardPage / SystemAdminPage directly, the CSS disappeared silently. Fix: each page component imports its own CSS at the top — DashboardPage needs both admin.css (.dashboard-page, .dashboard-hero, .dashboard-metrics, .dashboard-actions) and dashboard.css (.dashboard-grid). UserManagementPage / ProjectManagementPage / SystemAdminPage only need admin.css. Don't put CSS imports in route files; let the components own their styles. SchedulePage already follows this pattern with schedule.css.
  • Module-level zustand store + bindApi(api) for auth-dependent APIs. Keep a module-level _api ref; expose bindScriptWorkspaceApi(api); layout calls it in render body (not useEffect([api])). Actions read _api internally — no api param on every call. Pair the render-body bind with a separate empty-deps useEffect(() => () => bindScriptWorkspaceApi(null), []) for unmount cleanup only.
    • Why render body, not useEffect([api]): React effect order on deps change is parent cleanup → child effect → parent effect. With useEffect([api]), the parent's cleanup wipes _api to null before child effects (e.g. ScriptsPage's useEffect([workspaceId]) calling load()) run, producing the script workspace API 未绑定 race whenever currentWorkspace.workspace_id changes. Render-body binding runs synchronously during the parent's render, which happens before the child's render and effects, so _api is always current by the time child code touches the store.
  • Lifecycle hooks belong in the layout, not in route components. Heartbeats (active edit session + cached sessions), the 10-min cleanup timer, and beforeunload lock-release must mount at the layout level — navigating to /schedules otherwise unmounts them and cached locks expire. Pattern: store exposes tickHeartbeats() / tickCleanup() / releaseActiveOnUnload(); the layout hook just owns the setInterval and addEventListener.
  • Route components with their own internal state must remount on workspace/user change. SchedulesPage / SystemAdminPage / UserManagementPage / ProjectManagementPage keep useState for fetched data (schedules, artifacts, selectedSchedule, employees, projects). When currentWorkspace or user changes, the api reference updates but the cached state does not — the UI shows the previous workspace's data. Pattern: route wrappers set key={\${user?.user_id ?? "anon"}-${currentWorkspace?.workspace_id ?? "none"}`}on the page component to force React to unmount and remount, resetting all internal state and re-runninguseEffectdata fetches.ScriptsPagedoesn't need this — its store-backed state is reset viascriptWorkspaceStore.reset()` on workspace change.
  • { current: T | null } module-level handle for non-subscribing consumers. Sidebar reads "is there an active edit session?" without subscribing to the store — expose a plain { current: ... } object at module scope and update it synchronously inside the store's setEditSession action.
  • zustand StateCreator enforces declared action signatures. Declaring loadLatestVersion: () => Promise<void> while accidentally returning a cleanup function from the implementation makes tsc reject the whole store with TS2345. Match the declared type exactly.
  • Nested routes in React Router v8. Use route("", "layout.tsx", [route("x", "x.tsx"), ...]) from @react-router/dev/routes. URLs stay flat; the layout renders <Outlet />. Don't use route("*", ...) as a wildcard — it skips the nested children config.
  • Route ids are derived from file paths. The same file cannot be referenced by two route entries (index("X.tsx") + route("y", "X.tsx")duplicate route id). To make / redirect to /workbench, create a tiny RootIndex.tsx that renders <Navigate to="/workbench" replace /> rather than reusing DashboardRoute.tsx.
  • An unmatched nested child leaves <Outlet /> blank → white screen inside the layout. Always cover / either with index(...) or by letting the parent layout <Navigate> on a location check. /login is the only top-level path that escapes this trap.
  • pnpm typecheck runs react-router typegen && tsc. Type errors from the generated +types/... files surface here too. New route files must be registered in routes.ts first.

Frontend coupling

  • UserManagementPage.tsx and api.ts still call /api/v1/admin/employees. Don't migrate them in the same change as a /api/v1/platform/employees addition — the contract surface is intentionally duplicated.