Conversation
WalkthroughAdds version and migration management features across UI and server: new FastAPI endpoints for version and migrations, React UI cards and banners with query hooks, services, and types, plus SQL migrations and tracking table. Updates Settings page and README. Mounts migration folder in Docker. Adds tests for APIs/services and updates reset scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Settings UI
participant VersionHooks as useVersionQueries
participant UIService as versionService.ts (UI)
participant API as /api/version
participant Svc as VersionService (server)
participant GitHub as GitHub API
User->>Settings UI: Open Settings
Settings UI->>VersionHooks: useVersionCheck()
VersionHooks->>UIService: fetch checkVersion()
UIService->>API: GET /api/version/check (If-None-Match)
API->>Svc: check_for_updates()
alt Cache valid
Svc-->>API: cached latest release
else Fetch
Svc->>GitHub: GET /repos/:owner/:repo/releases/latest
GitHub-->>Svc: release JSON
Svc-->>API: latest + compare current
end
API-->>UIService: 200 with ETag or 304
UIService-->>VersionHooks: data/error
VersionHooks-->>Settings UI: {update_available, versions}
Settings UI-->>User: Show UpdateBanner/StatusCard
sequenceDiagram
autonumber
actor Admin
participant Settings UI
participant MigHooks as useMigrationQueries
participant UIMigSvc as migrationService.ts (UI)
participant API as /api/migrations
participant MigSvc as MigrationService (server)
participant FS as migration/*.sql
participant DB as Supabase (archon_migrations)
Admin->>Settings UI: Open Migrations section
Settings UI->>MigHooks: useMigrationStatus()
MigHooks->>UIMigSvc: getMigrationStatus()
UIMigSvc->>API: GET /api/migrations/status (If-None-Match)
API->>MigSvc: get_migration_status()
par Read applied
MigSvc->>DB: SELECT from archon_migrations
DB-->>MigSvc: applied list
and Scan filesystem
MigSvc->>FS: Read versioned .sql files
FS-->>MigSvc: files contents/checksums
end
MigSvc-->>API: {pending, applied, flags, counts, current_version}
API-->>UIMigSvc: 200 with ETag
UIMigSvc-->>MigHooks: data
MigHooks-->>Settings UI: status
Settings UI-->>Admin: Show pending banner/modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (19)
python/src/server/config/version.py (1)
7-11: Consider using a more dynamic versioning approach.While hardcoding the version is straightforward, consider whether this should be derived from a package.json, setup.py, or git tags to maintain consistency across the project and reduce manual update requirements.
If you want to maintain version consistency automatically, consider:
# Option 1: Read from a VERSION file def get_version(): try: with open("VERSION", "r") as f: return f.read().strip() except FileNotFoundError: return "0.1.0" # fallback ARCHON_VERSION = get_version() # Option 2: Use importlib.metadata for installed packages try: from importlib.metadata import version ARCHON_VERSION = version("archon") except ImportError: ARCHON_VERSION = "0.1.0"README.md (1)
215-221: Mention “Copy All” option to reduce frictionSince the Pending Migrations modal supports copying all SQL at once, consider adding a bullet: “Optionally, click ‘Copy All Migrations’ and paste into Supabase to run in order.”
- - Click on each migration to view and copy the SQL - - Run the SQL scripts in your Supabase SQL editor in the order shown + - Click on each migration to view and copy the SQL + - Or click “Copy All Migrations” to copy them in order + - Run the SQL scripts in your Supabase SQL editor in the order shownarchon-ui-main/src/features/settings/version/components/UpdateBanner.tsx (2)
57-63: Persist dismissal across sessionsUse localStorage so the banner stays dismissed after a reload.
- const [isDismissed, setIsDismissed] = React.useState(false); + const [isDismissed, setIsDismissed] = React.useState<boolean>(() => { + return window.localStorage.getItem("archon:updateBanner:dismissed") === "1"; + }); + const handleDismiss = () => { + window.localStorage.setItem("archon:updateBanner:dismissed", "1"); + setIsDismissed(true); + }; ... - <button type="button" - onClick={() => setIsDismissed(true)} + <button type="button" + onClick={handleDismiss} className="p-2 hover:bg-gray-700/50 rounded-lg transition-colors" aria-label="Dismiss update banner" >
19-27: Announce updates to screen readersAdd role=alert and aria-live for accessible status changes.
- <motion.div + <motion.div + role="alert" + aria-live="polite"archon-ui-main/src/features/settings/version/types/index.ts (1)
30-35: Type shape for error stateIf you later use VersionStatus with React Query results, consider
unknown | Errorto match default generics, though current usage is fine.-export interface VersionStatus { +export interface VersionStatus { isLoading: boolean; - error: Error | null; + error: unknown | null; data: VersionCheckResponse | null; lastChecked: Date | null; }migration/0.1.0/DB_UPGRADE_INSTRUCTIONS.md (2)
144-148: Normalize docker command spellingElsewhere you use
docker-compose restart. Preferdocker compose restartto stay consistent with Compose v2.- docker-compose restart + docker compose restart
112-133: Container name variability“supabase-db” may differ per setup. Consider a note: replace with your DB container name from
docker ps.-# Copy migrations to container -docker cp 001_add_source_url_display_name.sql supabase-db:/tmp/ +## Copy migrations to your DB container (replace 'supabase-db' with your container name) +docker cp 001_add_source_url_display_name.sql supabase-db:/tmp/archon-ui-main/src/features/settings/migrations/services/migrationService.ts (1)
12-20: Pass AbortSignal to support query cancellationWire React Query’s signal into fetches to prevent wasted work on tab hides/navigations.
- async getMigrationStatus(): Promise<MigrationStatusResponse> { + async getMigrationStatus(signal?: AbortSignal): Promise<MigrationStatusResponse> { try { - const response = await callAPIWithETag("/api/migrations/status"); + const response = await callAPIWithETag("/api/migrations/status", { signal }); return response as MigrationStatusResponse; } catch (error) { console.error("Error getting migration status:", error); throw error; } },Apply the same to
getMigrationHistoryandgetPendingMigrations, and in hooks:queryFn: ({ signal }) => migrationService.getMigrationStatus(signal).python/tests/server/services/test_migration_service.py (1)
5-11: Clean up unused imports
datetimeandMockappear unused; Ruff will flag these.-from datetime import datetime ... -from unittest.mock import AsyncMock, MagicMock, Mock, patch +from unittest.mock import AsyncMock, MagicMock, patchmigration/0.1.0/006_ollama_create_indexes_optional.sql (2)
10-11: Consider performance implications of maintenance_work_mem settingSetting
maintenance_work_memto 512MB might be insufficient for large tables or could exceed available memory on smaller instances. The fixed value doesn't account for system resources.Consider making this configurable or documenting minimum system requirements:
-SET maintenance_work_mem = '512MB'; +-- Adjust based on available system memory (recommended: 10-25% of total RAM) +-- Minimum: 256MB, Recommended: 512MB-2GB for production datasets +SET maintenance_work_mem = '512MB';
18-19: Consider tuning the 'lists' parameter for IVFFLAT indexesThe
lists = 100parameter is hardcoded for all IVFFLAT indexes. This value affects the trade-off between index build time, query speed, and recall accuracy. The optimal value depends on the dataset size.Consider making the
listsparameter dynamic based on expected dataset size:
- For small datasets (< 100K vectors):
lists = 100is fine- For medium datasets (100K - 1M vectors):
lists = sqrt(n) / 2where n is the number of rows- For large datasets (> 1M vectors):
lists = sqrt(n)up to a maximum of 5000You might want to document this guidance or make it configurable through an environment variable.
archon-ui-main/src/features/settings/migrations/types/index.ts (1)
8-8: Consider using Date type for timestamp fieldThe
applied_atfield is typed asstring, but it represents a timestamp. Consider usingDatetype or a branded type for better type safety and to enable date operations in the UI.export interface MigrationRecord { version: string; migration_name: string; - applied_at: string; + applied_at: string; // ISO 8601 datetime string checksum?: string | null; }Or even better, parse it as a Date in the service layer:
- applied_at: string; + applied_at: Date;archon-ui-main/src/features/settings/version/components/VersionStatusCard.tsx (2)
13-17: Consider handling mutation errors in refresh flowThe
handleRefreshClickfunction doesn't handle potential errors from the cache clear mutation. Users might not be aware if the cache clear fails.Add error handling for better user feedback:
const handleRefreshClick = async () => { - // Clear cache and then refetch - await clearCache.mutateAsync(); - refetch(); + try { + // Clear cache and then refetch + await clearCache.mutateAsync(); + refetch(); + } catch (error) { + console.error("Failed to clear cache:", error); + // Still attempt refetch even if cache clear fails + refetch(); + } };
31-31: Remove unnecessary type attribute on buttonThe
type="button"attribute is the default for button elements and can be omitted for cleaner code.- <button type="button" + <button onClick={handleRefreshClick}python/tests/server/services/test_version_service.py (1)
224-234: Consider extracting version comparison tests to a separate test fileThe
test_is_newer_versionfunction tests thesemantic_versionutility, not theVersionService. This should be in a separate test file for the semantic_version module.Consider moving this test to a new file
test_semantic_version.py:# python/tests/server/utils/test_semantic_version.py """Tests for semantic version utilities.""" import pytest from src.server.utils.semantic_version import is_newer_version, compare_versions, parse_version def test_is_newer_version(): """Test version comparison logic.""" assert is_newer_version("1.0.0", "2.0.0") is True assert is_newer_version("2.0.0", "1.0.0") is False # ... rest of the testspython/src/server/api_routes/migration_api.py (1)
145-146: Consider adding ETag support to pending migrations endpointFor consistency with the other endpoints, consider adding ETag support to the
/pendingendpoint.@router.get("/pending", response_model=list[PendingMigration]) -async def get_pending_migrations(): +async def get_pending_migrations(response: Response, if_none_match: str | None = Header(None)):Then add ETag generation and checking logic similar to the other endpoints after line 166.
python/src/server/services/migration_service.py (2)
39-39: Security consideration: MD5 is not cryptographically secureWhile MD5 is acceptable for checksums, consider documenting that these checksums are for integrity checking only, not security.
Consider adding a comment:
def _calculate_checksum(self, content: str) -> str: """Calculate MD5 checksum of migration content. Note: MD5 is used for integrity checking only, not for security purposes. """ return hashlib.md5(content.encode()).hexdigest()
156-156: Potential issue withPath.cwd()in Docker environmentsUsing
Path.cwd()may not work correctly in Docker if the working directory differs from expectations. Consider usingself._migrations_diras the base instead.- file_path=str(sql_file.relative_to(Path.cwd())), + file_path=str(sql_file.relative_to(self._migrations_dir.parent)),migration/0.1.0/005_ollama_create_functions.sql (1)
17-30: Consider consolidating duplicate dimension logicThe dimension-to-column mapping logic is duplicated in
get_embedding_column_nameand both search functions. While not critical, consider using the helper function internally.You could refactor the search functions to use
get_embedding_column_name:- CASE embedding_dimension - WHEN 384 THEN embedding_column := 'embedding_384'; - WHEN 768 THEN embedding_column := 'embedding_768'; - WHEN 1024 THEN embedding_column := 'embedding_1024'; - WHEN 1536 THEN embedding_column := 'embedding_1536'; - WHEN 3072 THEN embedding_column := 'embedding_3072'; - ELSE RAISE EXCEPTION 'Unsupported embedding dimension: %', embedding_dimension; - END CASE; + embedding_column := get_embedding_column_name(embedding_dimension);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
README.md(1 hunks)archon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsx(1 hunks)archon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsx(1 hunks)archon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts(1 hunks)archon-ui-main/src/features/settings/migrations/services/migrationService.ts(1 hunks)archon-ui-main/src/features/settings/migrations/types/index.ts(1 hunks)archon-ui-main/src/features/settings/version/components/UpdateBanner.tsx(1 hunks)archon-ui-main/src/features/settings/version/components/VersionStatusCard.tsx(1 hunks)archon-ui-main/src/features/settings/version/hooks/useVersionQueries.ts(1 hunks)archon-ui-main/src/features/settings/version/services/versionService.ts(1 hunks)archon-ui-main/src/features/settings/version/types/index.ts(1 hunks)archon-ui-main/src/pages/SettingsPage.tsx(4 hunks)docker-compose.yml(1 hunks)migration/0.1.0/003_ollama_add_columns.sql(1 hunks)migration/0.1.0/004_ollama_migrate_data.sql(1 hunks)migration/0.1.0/005_ollama_create_functions.sql(1 hunks)migration/0.1.0/006_ollama_create_indexes_optional.sql(1 hunks)migration/0.1.0/008_add_migration_tracking.sql(1 hunks)migration/0.1.0/DB_UPGRADE_INSTRUCTIONS.md(1 hunks)migration/DB_UPGRADE_INSTRUCTIONS.md(0 hunks)migration/RESET_DB.sql(2 hunks)migration/complete_setup.sql(1 hunks)migration/upgrade_database.sql(0 hunks)migration/validate_migration.sql(0 hunks)python/src/server/api_routes/migration_api.py(1 hunks)python/src/server/api_routes/version_api.py(1 hunks)python/src/server/config/version.py(1 hunks)python/src/server/main.py(2 hunks)python/src/server/services/migration_service.py(1 hunks)python/src/server/services/version_service.py(1 hunks)python/src/server/utils/semantic_version.py(1 hunks)python/tests/server/api_routes/test_migration_api.py(1 hunks)python/tests/server/api_routes/test_version_api.py(1 hunks)python/tests/server/services/test_migration_service.py(1 hunks)python/tests/server/services/test_version_service.py(1 hunks)
💤 Files with no reviewable changes (3)
- migration/DB_UPGRADE_INSTRUCTIONS.md
- migration/upgrade_database.sql
- migration/validate_migration.sql
🧰 Additional context used
📓 Path-based instructions (11)
archon-ui-main/src/features/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
archon-ui-main/src/features/**/*.{ts,tsx}: Use TanStack Query for all data fetching (no prop drilling); use smart HTTP polling (no WebSockets)
Biome formatting in features: 120-character line length, double quotes, and trailing commas
Apply Tron-inspired glassmorphism styling with Tailwind for feature UIUse Biome in features: 120 character line length, double quotes, and trailing commas
Files:
archon-ui-main/src/features/settings/migrations/types/index.tsarchon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsxarchon-ui-main/src/features/settings/version/components/VersionStatusCard.tsxarchon-ui-main/src/features/settings/migrations/services/migrationService.tsarchon-ui-main/src/features/settings/version/types/index.tsarchon-ui-main/src/features/settings/version/components/UpdateBanner.tsxarchon-ui-main/src/features/settings/version/services/versionService.tsarchon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsxarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
archon-ui-main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Disallow implicit any in TypeScript
archon-ui-main/src/**/*.{ts,tsx}: Frontend TypeScript must use strict mode with no implicit any
Use TanStack Query for all data fetching; avoid prop drilling
Use database values directly in the frontend; avoid mapping layers between BE and FE types
Files:
archon-ui-main/src/features/settings/migrations/types/index.tsarchon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsxarchon-ui-main/src/features/settings/version/components/VersionStatusCard.tsxarchon-ui-main/src/features/settings/migrations/services/migrationService.tsarchon-ui-main/src/features/settings/version/types/index.tsarchon-ui-main/src/features/settings/version/components/UpdateBanner.tsxarchon-ui-main/src/features/settings/version/services/versionService.tsarchon-ui-main/src/pages/SettingsPage.tsxarchon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsxarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
archon-ui-main/src/**/*.{ts,tsx,py}
📄 CodeRabbit inference engine (CLAUDE.md)
In code comments, avoid meta terms like SIMPLIFIED, ENHANCED, LEGACY, CHANGED, REMOVED; comment only on functionality and reasoning (do not mention beta/global rules)
Files:
archon-ui-main/src/features/settings/migrations/types/index.tsarchon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsxarchon-ui-main/src/features/settings/version/components/VersionStatusCard.tsxarchon-ui-main/src/features/settings/migrations/services/migrationService.tsarchon-ui-main/src/features/settings/version/types/index.tsarchon-ui-main/src/features/settings/version/components/UpdateBanner.tsxarchon-ui-main/src/features/settings/version/services/versionService.tsarchon-ui-main/src/pages/SettingsPage.tsxarchon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsxarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
python/src/server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/server/**/*.py: Never accept corrupted data in batch/continuation paths: skip failed items entirely (e.g., do not store zero embeddings, null FKs, or malformed JSON)
Use specific exception types; avoid catching bare Exception
Preserve full stack traces in logs (use exc_info=True with Python logging)
Never return None to indicate failure; raise an exception with details
Authentication/authorization failures must halt the operation and be clearly surfaced
Service startup, missing configuration, database connection, or critical dependency failures should crash fast with clear errors
During crawling/batch/background tasks and WebSocket events, continue processing other items but log failures with context
Include context (operation intent, relevant IDs/URLs/data) in error messages
Pydantic should raise on data corruption or validation errors; do not accept invalid inputs
Files:
python/src/server/config/version.pypython/src/server/main.pypython/src/server/utils/semantic_version.pypython/src/server/api_routes/version_api.pypython/src/server/api_routes/migration_api.pypython/src/server/services/version_service.pypython/src/server/services/migration_service.py
python/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/**/*.py: Target Python 3.12 with a 120-character line length
Use Ruff for linting (errors, warnings, unused imports)
Files:
python/src/server/config/version.pypython/src/server/main.pypython/src/server/utils/semantic_version.pypython/tests/server/api_routes/test_migration_api.pypython/src/server/api_routes/version_api.pypython/tests/server/services/test_version_service.pypython/src/server/api_routes/migration_api.pypython/tests/server/services/test_migration_service.pypython/tests/server/api_routes/test_version_api.pypython/src/server/services/version_service.pypython/src/server/services/migration_service.py
python/src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use MyPy for type checking to ensure type safety
python/src/**/*.py: On service startup, missing configuration, DB connection failures, auth/authorization failures, critical dependency outages, or invalid/corrupting data: fail fast and bubble errors
For batch processing, background tasks, WebSocket events, optional features, and external API calls: continue processing but log errors (with retries/backoff for APIs)
Never accept or persist corrupted data; skip failed items entirely (e.g., zero embeddings, null FKs, malformed JSON)
Error messages must include operation context, IDs/URLs, use specific exception types, preserve full stack traces (logging with exc_info=True), and avoid returning None/null—raise exceptions instead; for batches report success counts and detailed failures
Backend code targets Python 3.12 and adheres to a 120 character line length
Use Ruff for linting (errors, warnings, unused imports) in backend code
Use Mypy for static type checking in backend code
Files:
python/src/server/config/version.pypython/src/server/main.pypython/src/server/utils/semantic_version.pypython/src/server/api_routes/version_api.pypython/src/server/api_routes/migration_api.pypython/src/server/services/version_service.pypython/src/server/services/migration_service.py
python/src/server/main.py
📄 CodeRabbit inference engine (CLAUDE.md)
Register FastAPI exception handlers in the main application (search for @app.exception_handler)
Files:
python/src/server/main.py
{python/src/server/exceptions.py,python/src/server/main.py}
📄 CodeRabbit inference engine (CLAUDE.md)
Define custom exceptions in exceptions.py and ensure handlers are wired in main.py
Files:
python/src/server/main.py
python/tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Write backend tests with Pytest (including async support)
Files:
python/tests/server/api_routes/test_migration_api.pypython/tests/server/services/test_version_service.pypython/tests/server/services/test_migration_service.pypython/tests/server/api_routes/test_version_api.py
python/src/server/api_routes/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place HTTP route handlers in the API routes directory
Files:
python/src/server/api_routes/version_api.pypython/src/server/api_routes/migration_api.py
python/src/server/services/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/src/server/services/**/*.py: For batch operations, report both success count and a detailed failure list
On external API calls, retry with exponential backoff and fail with a clear message after retries
Place business logic in service layer modules (API Route → Service → Database pattern)
Files:
python/src/server/services/version_service.pypython/src/server/services/migration_service.py
🧠 Learnings (6)
📚 Learning: 2025-09-19T10:32:55.557Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-19T10:32:55.557Z
Learning: Applies to archon-ui-main/src/features/*/types/**/*.{ts,tsx} : Define shared types under src/features/[feature]/types
Applied to files:
archon-ui-main/src/features/settings/migrations/types/index.tsarchon-ui-main/src/features/settings/version/types/index.ts
📚 Learning: 2025-09-19T10:31:54.277Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-19T10:31:54.277Z
Learning: Applies to archon-ui-main/src/features/*/types/**/*.{ts,tsx} : Define feature-specific TypeScript types in the feature’s types directory
Applied to files:
archon-ui-main/src/features/settings/migrations/types/index.tsarchon-ui-main/src/features/settings/version/types/index.ts
📚 Learning: 2025-09-19T10:32:55.557Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-19T10:32:55.557Z
Learning: Applies to archon-ui-main/src/features/*/hooks/**/*.{ts,tsx} : Use feature-scoped TanStack Query hooks in src/features/[feature]/hooks
Applied to files:
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
📚 Learning: 2025-09-19T10:31:54.277Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-19T10:31:54.277Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Use TanStack Query for all data fetching (no prop drilling); use smart HTTP polling (no WebSockets)
Applied to files:
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
📚 Learning: 2025-09-19T10:31:54.277Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-19T10:31:54.277Z
Learning: Applies to archon-ui-main/src/features/*/hooks/**/*.{ts,tsx} : Place TanStack Query hooks in the feature’s hooks directory
Applied to files:
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
📚 Learning: 2025-09-19T10:32:55.557Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-19T10:32:55.557Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TanStack Query for all data fetching; avoid prop drilling
Applied to files:
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.tsarchon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts
🧬 Code graph analysis (19)
archon-ui-main/src/features/settings/migrations/types/index.ts (2)
python/src/server/api_routes/migration_api.py (4)
MigrationRecord(17-23)PendingMigration(26-33)MigrationStatusResponse(36-45)MigrationHistoryResponse(48-53)python/src/server/services/migration_service.py (2)
MigrationRecord(16-24)PendingMigration(27-39)
archon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsx (2)
archon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts (1)
useMigrationStatus(23-33)archon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsx (1)
PendingMigrationsModal(19-195)
archon-ui-main/src/features/settings/version/components/VersionStatusCard.tsx (2)
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.ts (2)
useVersionCheck(22-33)useClearVersionCache(49-59)archon-ui-main/src/features/settings/version/services/versionService.ts (1)
clearCache(38-48)
archon-ui-main/src/features/settings/migrations/services/migrationService.ts (2)
archon-ui-main/src/features/settings/migrations/types/index.ts (3)
MigrationStatusResponse(20-28)MigrationHistoryResponse(30-34)PendingMigration(12-18)archon-ui-main/src/features/shared/apiWithEtag.ts (1)
callAPIWithETag(43-120)
archon-ui-main/src/features/settings/version/types/index.ts (1)
python/src/server/api_routes/version_api.py (3)
ReleaseAsset(18-25)VersionCheckResponse(28-39)CurrentVersionResponse(42-46)
archon-ui-main/src/features/settings/version/components/UpdateBanner.tsx (1)
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.ts (1)
useVersionCheck(22-33)
archon-ui-main/src/features/settings/version/services/versionService.ts (3)
archon-ui-main/src/features/settings/version/types/index.ts (2)
VersionCheckResponse(13-23)CurrentVersionResponse(25-28)python/src/server/api_routes/version_api.py (2)
VersionCheckResponse(28-39)CurrentVersionResponse(42-46)archon-ui-main/src/features/shared/apiWithEtag.ts (1)
callAPIWithETag(43-120)
archon-ui-main/src/pages/SettingsPage.tsx (3)
archon-ui-main/src/features/settings/version/components/UpdateBanner.tsx (1)
UpdateBanner(10-69)archon-ui-main/src/features/settings/version/components/VersionStatusCard.tsx (1)
VersionStatusCard(9-98)archon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsx (1)
MigrationStatusCard(11-132)
python/tests/server/api_routes/test_migration_api.py (2)
python/tests/server/services/test_migration_service.py (2)
migration_service(21-27)test_get_migration_status_bootstrap_required(231-258)python/src/server/services/migration_service.py (1)
get_applied_migrations(99-119)
python/src/server/api_routes/version_api.py (3)
python/src/server/utils/etag_utils.py (2)
check_etag(27-42)generate_etag(8-24)archon-ui-main/src/features/settings/version/types/index.ts (3)
ReleaseAsset(5-11)VersionCheckResponse(13-23)CurrentVersionResponse(25-28)python/src/server/services/version_service.py (2)
check_for_updates(89-153)clear_cache(155-158)
python/tests/server/services/test_version_service.py (2)
python/src/server/services/version_service.py (2)
VersionService(15-158)get_latest_release(31-87)python/src/server/utils/semantic_version.py (1)
is_newer_version(92-107)
python/src/server/api_routes/migration_api.py (2)
python/src/server/utils/etag_utils.py (2)
check_etag(27-42)generate_etag(8-24)python/src/server/services/migration_service.py (5)
MigrationRecord(16-24)PendingMigration(27-39)get_migration_status(191-229)get_applied_migrations(99-119)get_pending_migrations(164-189)
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.ts (4)
archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (1)
useSmartPolling(13-72)archon-ui-main/src/features/settings/version/types/index.ts (1)
VersionCheckResponse(13-23)archon-ui-main/src/features/settings/version/services/versionService.ts (1)
versionService(8-49)archon-ui-main/src/features/shared/queryPatterns.ts (1)
STALE_TIMES(18-25)
archon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsx (3)
archon-ui-main/src/features/settings/migrations/types/index.ts (1)
PendingMigration(12-18)archon-ui-main/src/features/ui/hooks/useToast.ts (1)
useToast(26-32)archon-ui-main/src/features/shared/utils/clipboard.ts (1)
copyToClipboard(17-91)
python/tests/server/services/test_migration_service.py (1)
python/src/server/services/migration_service.py (2)
MigrationService(42-229)get_applied_migrations(99-119)
python/tests/server/api_routes/test_version_api.py (1)
python/tests/server/api_routes/test_migration_api.py (1)
client(17-19)
python/src/server/services/version_service.py (3)
python/src/server/utils/semantic_version.py (1)
is_newer_version(92-107)python/src/server/api_routes/version_api.py (1)
check_for_updates(54-95)python/tests/server/services/test_version_service.py (1)
version_service(17-23)
python/src/server/services/migration_service.py (1)
python/src/server/services/client_manager.py (1)
get_supabase_client(15-43)
archon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts (4)
archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (1)
useSmartPolling(13-72)archon-ui-main/src/features/settings/migrations/types/index.ts (3)
MigrationStatusResponse(20-28)MigrationHistoryResponse(30-34)PendingMigration(12-18)archon-ui-main/src/features/settings/migrations/services/migrationService.ts (1)
migrationService(8-47)archon-ui-main/src/features/shared/queryPatterns.ts (1)
STALE_TIMES(18-25)
🔇 Additional comments (70)
archon-ui-main/src/features/settings/version/services/versionService.ts (5)
5-6: LGTM: Clean service architecture with proper TypeScript imports.The imports follow the coding guidelines with appropriate type imports and the service layer pattern aligns well with the TanStack Query integration shown in the relevant code snippets.
8-20: Good error handling pattern but consider TanStack Query compatibility.The error handling approach here re-throws errors for TanStack Query to handle, which aligns with TanStack Query's requirement that "query function must throw or return a rejected Promise" and that "Any error that is thrown in the query function will be persisted on the error state of the query."
25-33: LGTM: Consistent service pattern implementation.The
getCurrentVersionmethod follows the same error handling pattern ascheckVersion, maintaining consistency across the service.
38-48: Good cache management functionality with proper HTTP method.The use of POST method for cache clearing is appropriate, and the error handling follows the established pattern. This aligns with REST conventions where POST is used for actions that modify server state.
12-19: Verify /api/version endpoint consistency
- Backend exposes /api/version/check, /api/version/current, and /api/version/clear-cache (python/src/server/api_routes/version_api.py).
- Frontend archon-ui-main/src/features/settings/version/services/versionService.ts calls /api/version/check — this matches the backend. Ripgrep in the sandbox failed to scan TS/TSX files ("unrecognized file type: tsx"); run a repo-wide search for '/api/version' in the UI code to confirm there are no other hardcoded endpoints.
python/src/server/utils/semantic_version.py (3)
8-49: Solid semantic version parsing with good edge case handling.The implementation handles various version formats well and includes proper error handling. The support for incomplete versions like "1.0" is a nice touch for real-world usage.
52-89: Correct semantic version comparison logic.The comparison logic properly handles the precedence rules where release versions (without prerelease) are considered newer than prerelease versions, and lexicographic comparison for prerelease strings is appropriate.
92-107: Good defensive programming with graceful error fallback.The function correctly returns
Falsewhen version parsing fails, which is a safe default behavior that won't incorrectly indicate updates are available.migration/0.1.0/003_ollama_add_columns.sql (3)
6-7: Good performance consideration for large schema changes.Setting
maintenance_work_mem = '256MB'is appropriate for adding multiple columns with indexes, especially for vector columns that may require significant memory during operations.
11-31: Well-structured multi-dimensional embedding support.The column additions are properly structured with
IF NOT EXISTSclauses to ensure idempotent migrations. The range of embedding dimensions (384, 768, 1024, 1536, 3072) covers common model sizes effectively.
9-33: VECTOR extension present; no action required.migration/complete_setup.sql includes
CREATE EXTENSION IF NOT EXISTS vector;(around lines 15–17) and creates ivfflat indexes for embedding_1024 and embedding_1536; the file also documents that embedding_3072 cannot be indexed due to the pgvector 2000‑dimension limit.docker-compose.yml (1)
38-38: LGTM: Proper volume mount for migration tracking.The volume mount
./migration:/app/migrationproperly exposes migration files to the container, enabling the migration service to discover and track applied migrations. This aligns well with the migration tracking functionality introduced in this PR.migration/RESET_DB.sql (2)
67-70: LGTM: Consistent migration table cleanup.The addition of migration tracking table policies to the cleanup script is appropriate and maintains consistency with the new migration tracking infrastructure.
182-184: LGTM: Complete migration tracking table cleanup.Including the
archon_migrationstable in the reset script ensures complete cleanup of the migration tracking infrastructure.python/src/server/main.py (2)
26-26: LGTM: Clean router imports following established patterns.The imports follow the existing pattern and maintain alphabetical ordering.
Also applies to: 30-30
191-192: LGTM: Proper router registration.The router registration follows the established pattern and maintains consistency with other API modules.
python/tests/server/api_routes/test_migration_api.py (5)
5-13: LGTM: Comprehensive test imports and setup.The imports cover all necessary testing components and follow Python testing best practices with proper fixtures and mocking.
22-77: Well-structured test fixtures with realistic data.The fixtures provide comprehensive test data covering both applied and pending migrations with realistic timestamps and checksums. The data structure properly reflects the domain models.
80-96: Comprehensive success case testing.The test properly validates all expected fields in the migration status response and ensures data integrity.
98-119: Good edge case coverage for bootstrap scenarios.Testing the bootstrap_required scenario is important for new installations and properly validates the migration system's behavior when no migrations table exists.
121-206: Excellent error handling and edge case coverage.The tests comprehensively cover error scenarios, empty states, and various success cases for all three endpoints. The error message validation ensures proper error propagation to the frontend.
README.md (1)
209-214: Upgrade step clarity looks goodRebuild-and-restart guidance is clear and correctly ordered after pulling. No blocking issues here.
archon-ui-main/src/features/settings/version/components/UpdateBanner.tsx (1)
10-17: Early-return gating reads wellGates on loading, error, dismissed, and update_available. Behavior matches intent.
archon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsx (1)
21-40: Overall structure and UX: LGTMGood use of TanStack Query, clear states, and accessible controls (labels, disabled states).
migration/0.1.0/DB_UPGRADE_INSTRUCTIONS.md (1)
79-83: Compose command consistencyUse “docker compose” (space) consistently; here it’s correct.
archon-ui-main/src/features/settings/migrations/services/migrationService.ts (1)
8-47: Service surface and error handling look goodEndpoints, typing, and rethrow semantics are reasonable. No blocking issues.
archon-ui-main/src/pages/SettingsPage.tsx (2)
114-116: Banner placement is sensibleRendering UpdateBanner above the header is a good UX call.
148-172: New sections integrate cleanlyCollapsible cards for “Version & Updates” and “Database Migrations” are consistent with existing UI patterns and follow the TanStack Query hooks.
archon-ui-main/src/features/settings/migrations/types/index.ts (1)
5-10: Good type alignment with backend modelsThe TypeScript interfaces correctly mirror the backend Pydantic models, ensuring type safety across the stack.
archon-ui-main/src/features/settings/version/components/VersionStatusCard.tsx (1)
1-98: Well-structured component with good UX patternsThe component properly implements loading states, error handling, and user feedback with appropriate visual indicators. The integration with TanStack Query hooks follows best practices.
python/tests/server/services/test_version_service.py (2)
1-234: Comprehensive test coverage with good practicesThe test suite thoroughly covers the VersionService functionality including caching behavior, error handling, and edge cases. Good use of fixtures and mocking.
149-149: No change required — timezone handling is correct.
python/src/server/services/version_service.py (lines 124-126) replaces "Z" with "+00:00" before datetime.fromisoformat, so the test's comparison to datetime.fromisoformat("2025-01-01T00:00:00+00:00") is valid.python/tests/server/api_routes/test_version_api.py (9)
15-18: LGTM!The test client fixture follows the established pattern from the migration tests and properly imports the application.
21-35: LGTM!Good mock data fixture that covers all expected version check response fields with realistic values.
37-50: LGTM!Well-structured test that properly mocks the version service and validates the expected response fields.
52-74: LGTM!Good test coverage for the no-update scenario, ensuring proper handling when current and latest versions match.
78-98: LGTM!Excellent test for ETag handling. It validates that when data changes, the API returns 200 with updated data instead of 304. This correctly tests the ETag cache invalidation behavior.
100-113: LGTM!Good error handling test that verifies graceful degradation when the version service fails, returning a safe default response structure.
115-123: LGTM!Simple but effective test for the current version endpoint.
125-137: LGTM!Good test for successful cache clearing with proper verification of the service method call.
139-147: LGTM!Appropriate error handling test for cache clearing failures, verifying 500 status and error detail.
archon-ui-main/src/features/settings/migrations/components/PendingMigrationsModal.tsx (5)
1-17: LGTM!Clean component interface following React best practices with appropriate props and proper typing.
19-48: Component implementation looks solid.The component follows React best practices with proper state management, async clipboard operations, and user feedback through toasts.
52-103: Excellent modal structure with Tron-inspired styling.The modal implementation properly uses AnimatePresence for smooth animations, includes accessibility features, and provides clear user instructions for migration workflow. The glassmorphism styling aligns with the design requirements.
105-174: Well-designed migration list with proper UX.The migration list implementation handles empty states gracefully, provides expandable SQL content, and includes proper copy functionality with visual feedback. The unique key generation using both version and name is appropriate.
176-195: Good footer with clear actions.The footer provides appropriate actions for closing the modal and refreshing migration status, maintaining consistent styling.
archon-ui-main/src/features/settings/version/hooks/useVersionQueries.ts (4)
1-16: LGTM!Clean query key factory implementation following TanStack Query best practices with proper hierarchical structure and TypeScript const assertions.
18-33: Excellent version check hook with smart polling.The hook properly implements smart polling for version checks with appropriate 5-minute intervals, uses the rare stale time, and disables retries for network errors - all sensible choices for version checking.
35-44: LGTM!Good current version hook that uses static stale time since version data rarely changes during runtime.
46-59: Well-implemented cache clearing mutation.The mutation properly invalidates all version queries on success, ensuring fresh data after cache clearing. Good use of the query client for cache management.
python/src/server/services/version_service.py (5)
15-22: LGTM!Good service initialization with reasonable cache TTL of 1 hour and proper instance variables.
23-30: LGTM!Clean cache validation logic with proper null checks and time comparison.
31-88: Excellent GitHub API integration with robust error handling.The method properly handles caching, GitHub API specifics (404 for no releases, proper headers), and provides fallback to cached data during failures. The timeout and error handling are comprehensive.
89-154: Well-structured update checking logic.The method correctly strips version prefixes, uses semantic version comparison, handles date parsing safely, and provides comprehensive error handling with safe defaults.
155-163: LGTM!Simple and effective cache clearing implementation with singleton export pattern.
python/src/server/api_routes/version_api.py (4)
17-47: LGTM!Well-defined Pydantic models with proper typing and field descriptions. The models correctly match the expected response structure.
53-96: Excellent ETag implementation with proper error handling.The endpoint correctly implements ETag caching with 304 responses, handles service errors gracefully by returning safe defaults, and maintains proper cache headers.
98-106: LGTM!Simple current version endpoint with appropriate response structure.
108-122: Good cache clearing endpoint with proper error handling.The endpoint appropriately handles service exceptions and returns proper HTTP status codes.
archon-ui-main/src/features/settings/migrations/hooks/useMigrationQueries.ts (4)
11-17: LGTM!Clean query key factory following established patterns with proper hierarchical structure.
19-33: Good migration status hook with smart polling.The hook uses appropriate 30-second polling intervals for migration status checks, which is reasonable for monitoring migration progress.
35-44: LGTM!Good migration history hook with longer stale time since history changes infrequently.
46-58: Good pending migrations hook with appropriate polling.The hook properly polls for pending migrations with the same 30-second interval as status checks, maintaining consistency.
migration/complete_setup.sql (1)
954-1009: LGTM!The migration tracking table and policies are well-designed with:
- Proper UUID primary key and unique constraints
- Appropriate indexes for performance
- Good RLS policies limiting authenticated users to read-only access
- Clear documentation via comments
- Idempotent policy creation with proper cleanup
The initial migration records correctly establish the baseline for migration tracking.
migration/0.1.0/008_add_migration_tracking.sql (4)
7-15: LGTM!Well-designed migration tracking table with proper constraints and data types. The unique constraint on (version, migration_name) prevents duplicate entries appropriately.
17-29: Good indexing and documentation.The indexes on version and applied_at (DESC) support efficient queries for migration status checks. The table and column comments provide clear documentation of purpose.
30-48: Excellent self-recording pattern.The migration correctly records itself and retroactively captures all previous migrations. This bootstrap approach ensures the tracking system is complete from the start.
50-65: Proper security implementation.The RLS policies are appropriate - service role gets full access for migration operations while authenticated users can only read the migration history. The idempotent policy creation prevents conflicts.
migration/0.1.0/005_ollama_create_functions.sql (1)
1-173: LGTM! Well-structured migration with proper transaction handlingThe migration properly uses transactions, provides backward compatibility, and includes defensive programming with dimension validation. The use of dynamic SQL with
format()and parameter binding is secure against SQL injection.python/src/server/services/migration_service.py (1)
70-82: Verify and secure the Supabase RPC "sql" before merge.
- Location: python/src/server/services/migration_service.py lines 70–82 (calls supabase.rpc("sql", ...)).
- Repo search returned no migration/docs for this RPC; provide the RPC function SQL (migration or DB), plus any GRANT/REVOKE statements.
- Confirm: does it accept arbitrary SQL from callers; is it SECURITY DEFINER or INVOKER; does it set an explicit search_path; what RLS applies to affected tables; who has EXECUTE (PUBLIC or limited roles); is any service_role key exposed to the frontend?
- If it executes arbitrary SQL or is callable by untrusted roles, remove or restrict it — replace with parameterized stored procedures and ensure only trusted backend roles can call it.
| {data?.has_pending && ( | ||
| <div className="mt-4 p-3 bg-yellow-500/10 border border-yellow-500/30 rounded-lg"> | ||
| <p className="text-yellow-400 text-sm mb-2"> | ||
| {data.bootstrap_required | ||
| ? "Initial database setup is required." | ||
| : `${data.pending_count} migration${data.pending_count > 1 ? "s" : ""} need to be applied.`} | ||
| </p> | ||
| <button type="button" | ||
| onClick={() => setIsModalOpen(true)} | ||
| className="px-3 py-1.5 bg-yellow-500/20 hover:bg-yellow-500/30 border border-yellow-500/50 rounded text-yellow-400 text-sm font-medium transition-colors" | ||
| > | ||
| View Pending Migrations | ||
| </button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Avoid sending full SQL in the status query; fetch only when needed
Status payload includes pending_migrations with full SQL, which can be large. Prefer lightweight status (counts/flags) and lazy‑load /pending when opening the modal.
- {data && (
- <PendingMigrationsModal
- isOpen={isModalOpen}
- onClose={() => setIsModalOpen(false)}
- migrations={data.pending_migrations}
- onMigrationsApplied={refetch}
- />
- )}
+ {/* Load pending migrations only when modal is open */}
+ {isModalOpen && <LazyPendingMigrations refetchStatus={refetch} />}Additional component (in this feature) to add:
// LazyPendingMigrations.tsx
import React from "react";
import { usePendingMigrations } from "../hooks/useMigrationQueries";
import { PendingMigrationsModal } from "./PendingMigrationsModal";
export function LazyPendingMigrations({ refetchStatus }: { refetchStatus: () => void }) {
const { data } = usePendingMigrations({ enabled: true });
const [open, setOpen] = React.useState(true);
return (
<PendingMigrationsModal
isOpen={open}
onClose={() => setOpen(false)}
migrations={data ?? []}
onMigrationsApplied={refetchStatus}
/>
);
}Update the backend /status to omit pending_migrations (keep counts) if it currently returns the SQL list.
🤖 Prompt for AI Agents
In
archon-ui-main/src/features/settings/migrations/components/MigrationStatusCard.tsx
around lines 96 to 110, the component currently renders a status payload that
may include full SQL for pending_migrations; change it to only show counts/flags
and lazy-load the full SQL when the user opens the modal: remove any usage of
data.pending_migrations here, keep data.has_pending, data.bootstrap_required and
data.pending_count for display, and onClick set a local state (e.g.,
setIsPendingModalOpen(true)) to mount a new LazyPendingMigrations component; add
the suggested LazyPendingMigrations component that calls usePendingMigrations({
enabled: true }) and controls the PendingMigrationsModal open state, and ensure
the modal's onClose unmounts it and onMigrationsApplied calls refetchStatus;
finally update the backend /status endpoint to stop returning the
pending_migrations SQL list (keep counts/flags) so the frontend status request
remains lightweight.
| export interface VersionCheckResponse { | ||
| current: string; | ||
| latest: string | null; | ||
| update_available: boolean; | ||
| release_url: string | null; | ||
| release_notes: string | null; | ||
| published_at: string | null; | ||
| check_error?: string | null; | ||
| assets?: ReleaseAsset[] | null; | ||
| author?: string | null; | ||
| } |
There was a problem hiding this comment.
Align assets typing with backend to avoid narrowing
Backend exposes assets: list[dict[str, Any]] | None, but FE narrows to ReleaseAsset[]. This can cause runtime/typing drift if fields vary.
-export interface VersionCheckResponse {
+export interface VersionCheckResponse {
current: string;
latest: string | null;
update_available: boolean;
release_url: string | null;
release_notes: string | null;
published_at: string | null;
check_error?: string | null;
- assets?: ReleaseAsset[] | null;
+ assets?: Array<ReleaseAsset | Record<string, unknown>> | null;
author?: string | null;
}Optionally, widen ReleaseAsset fields to optional if the API isn’t guaranteed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface VersionCheckResponse { | |
| current: string; | |
| latest: string | null; | |
| update_available: boolean; | |
| release_url: string | null; | |
| release_notes: string | null; | |
| published_at: string | null; | |
| check_error?: string | null; | |
| assets?: ReleaseAsset[] | null; | |
| author?: string | null; | |
| } | |
| export interface VersionCheckResponse { | |
| current: string; | |
| latest: string | null; | |
| update_available: boolean; | |
| release_url: string | null; | |
| release_notes: string | null; | |
| published_at: string | null; | |
| check_error?: string | null; | |
| assets?: Array<ReleaseAsset | Record<string, unknown>> | null; | |
| author?: string | null; | |
| } |
🤖 Prompt for AI Agents
In archon-ui-main/src/features/settings/version/types/index.ts around lines 13
to 23, the VersionCheckResponse.assets is typed as ReleaseAsset[] | null which
narrows the backend-provided list[dict[str, Any]] and can cause type/runtime
drift; change the assets typing to match the backend by making it assets?:
Array<Record<string, any>> | null (or assets?: Array<unknown> | null) so the
shape is not assumed, or alternatively relax the ReleaseAsset interface by
making all its fields optional; update any consuming code to validate/cast
fields at runtime if relying on specific keys.
- Include both providers_api from main and version/migration APIs from this branch - Both routers are now properly imported and registered
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
migration/complete_setup.sql (2)
969-971: DESC in single-column btree index is unnecessary.Postgres can use a plain btree for both ASC/DESC sorts; dropping DESC avoids a duplicate index definition footprint.
-CREATE INDEX IF NOT EXISTS idx_archon_migrations_applied_at ON archon_migrations(applied_at DESC); +CREATE INDEX IF NOT EXISTS idx_archon_migrations_applied_at ON archon_migrations(applied_at);
960-966: Future‑proof checksum and consider capturing actor.If you plan to store digests, 32 chars locks you to MD5. Suggest 64 to allow SHA‑256, and optionally record who applied the migration.
- checksum VARCHAR(32), + checksum VARCHAR(64), -- allow SHA-256 + -- optional: who applied the migration (JWT role if available) + applied_by TEXT DEFAULT COALESCE(NULLIF(current_setting('request.jwt.claims', true), '')::jsonb->>'role', NULL),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
migration/complete_setup.sql(1 hunks)python/src/server/main.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- python/src/server/main.py
🔇 Additional comments (1)
migration/complete_setup.sql (1)
981-991: Confirm seeded migration names exist on-disk and match migration service keysScan result: "No 0.1.0 SQL files found under migrations/ or migration/". Verify the eight seeded names in migration/complete_setup.sql (lines 981–991) exactly match the on‑disk filenames (extensionless vs .sql) in the per‑release migration directory your service uses and update the seed rows or relocate/rename the files to match.
| -- Service role has full access | ||
| CREATE POLICY "Allow service role full access to archon_migrations" ON archon_migrations | ||
| FOR ALL USING (auth.role() = 'service_role'); | ||
|
|
There was a problem hiding this comment.
Add WITH CHECK to RLS to avoid INSERT/UPDATE bypass.
For INSERT, USING is ignored; without WITH CHECK this policy effectively allows any role to insert when a matching policy exists. Gate writes explicitly to the service role.
Apply this diff:
-CREATE POLICY "Allow service role full access to archon_migrations" ON archon_migrations
- FOR ALL USING (auth.role() = 'service_role');
+CREATE POLICY "Allow service role full access to archon_migrations" ON archon_migrations
+ FOR ALL
+ USING (auth.role() = 'service_role')
+ WITH CHECK (auth.role() = 'service_role');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- Service role has full access | |
| CREATE POLICY "Allow service role full access to archon_migrations" ON archon_migrations | |
| FOR ALL USING (auth.role() = 'service_role'); | |
| -- Service role has full access | |
| CREATE POLICY "Allow service role full access to archon_migrations" ON archon_migrations | |
| FOR ALL | |
| USING (auth.role() = 'service_role') | |
| WITH CHECK (auth.role() = 'service_role'); |
🤖 Prompt for AI Agents
In migration/complete_setup.sql around lines 1001 to 1004, the row-level
security policy for archon_migrations uses only USING which is ignored for
INSERT/UPDATE and thus allows writes to bypass the intended restriction; update
the policy to include a WITH CHECK clause that enforces the same condition for
write operations (i.e., add WITH CHECK (auth.role() = 'service_role') so both
USING and WITH CHECK gate reads and writes to the service role).
|
Looks good, just missing Radix i will fix in follow up PR |
* Preparing migration folder for the migration alert implementation * Migrations and version APIs initial * Touching up update instructions in README and UI * Unit tests for migrations and version APIs * Splitting up the Ollama migration scripts * Removing temporary PRPs --------- Co-authored-by: Rasmus Widing <rasmus.widing@gmail.com>
The Settings page "Active" count was always 0 for web workflows because they are fire-and-forget (lock released immediately after dispatch). Query the DB for workflows with status='running' instead. Co-authored-by: Thomas Ritter <thomas.ritter@crownpeak.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oleam00#719) The Settings page "Active" count was always 0 for web workflows because they are fire-and-forget (lock released immediately after dispatch). Query the DB for workflows with status='running' instead. Co-authored-by: Thomas Ritter <thomas.ritter@crownpeak.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oleam00#719) The Settings page "Active" count was always 0 for web workflows because they are fire-and-forget (lock released immediately after dispatch). Query the DB for workflows with status='running' instead. Co-authored-by: Thomas Ritter <thomas.ritter@crownpeak.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pull Request
Summary
Added a migrations and version API so users can see when their Archon version is outdated or if there are database migrations that need to be run.
Changes Made
Type of Change
Affected Services
Testing
Test Evidence
# cd Archon/python && pytest -vUnit tests added for the migration and version APIs that are included with the above command.
Also did a bunch of testing starting the DB from scratch, deleting some migrations, rerunning migrations, etc. to make sure the migration alerts in the UI are always accurate and list the correct migrations that need to be run.
For the version API, I also temporarily mocked the GitHub API call to return a newer version to make sure the alerts are working properly in the UI.
Checklist
Breaking Changes
Right now users will be alerted of migrations that need to be run even if they have already run them since we are just now adding the archon_migrations table. This is okay though because all migration scripts can be rerun without a negative impact on the database.
Additional Notes
Planning for the 0.1.0 release next week for Archon where this PR will really come into effect. We will be tracking migrations for each release with migrations/[release]/ going forward now.
Summary by CodeRabbit
New Features
Documentation
Database
Chores