Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .claude/skills/gen_tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
**Action:**
Read `docs/architecture/[filename.md]`. Derive the complete test plan for this implementation. Then run it interactively, one step at a time — do not dump the full list and wait. Present each step, wait for the user to execute and report the result, then proceed.

**Presentation format (both phases):** Give each command its own small fenced code block — never combine multiple commands into one block. Most chat UIs put a copy button on each fenced block automatically, so one-command-per-block gives per-command copy ergonomics natively, no HTML/JS needed. Precede each command with a bold step label (e.g. `**06 · Pre-commit**`), follow it with an italicized `*Expect: ...*` line stating the observable success condition. Keep this entirely inline in chat — do not publish it as an Artifact. Test plans reference internal project details (branch names, env var names, infra topology) that don't need to leave the local conversation just for a nicer visual format; inline markdown is local, lightweight, and sufficient.
**Presentation format (both phases):** Give each command its own small fenced code block — never combine multiple commands into one block. Most chat UIs put a copy button on each fenced block automatically, so one-command-per-block gives per-command copy ergonomics natively, no HTML/JS needed. Precede each command with a bold step label (e.g. `**06 · Pre-commit**`), follow it with an italicized `*Expect: ...*` line stating the observable success condition. Keep this entirely inline in chat — do not publish it as an Artifact. Test plans reference internal project details (branch names, env var names, infra topology) that don't need to leave the local conversation just for a nicer visual format; inline markdown is local, lightweight, and sufficient. This format applies whenever steps are shown to the user — including when re-listing or recapping remaining/upcoming steps, not just when presenting the next step to execute. Never substitute a plain summary table for the full command+`Expect` format.

**SQL in QA steps:** When a QA step involves a SQL query, present the raw SQL in its own fenced block — for pasting directly into DBeaver or another SQL client — rather than wrapping it in a `docker compose exec db psql -c "..."` invocation. Easier to read and copy on its own than escaped inside a shell command.

---

Expand Down Expand Up @@ -33,6 +35,7 @@ Derive the exact commands from context:
never do this automatically as part of a QA step.
- `docker compose exec api uv run pytest --cov=src --cov-fail-under=90 -q`
- Sphinx locally: `DATABASE_URL="postgresql+psycopg2://dummy:dummy@localhost/dummy" REDIS_URL="redis://localhost:6379/0" uv run sphinx-build -b html docs docs/_build/html -W`
- Pre-commit locally: `uv run pre-commit run --all-files`. Before running this, check `git status --short` for `??` (untracked) entries. If any exist, stop and ask the user to commit them first — never stage or commit on their behalf. Pre-commit's `--all-files` only checks git-tracked files (confirmed empirically during doc 28's `genTests`), so running it against an untracked file gives a false "Passed" that doesn't reflect what will actually be pushed. Re-check any time a `genTests`-driven bug fix creates a new file — it needs to be committed again before the next pre-commit run means anything.

Wait for the user to confirm all CLI steps pass before proceeding to Phase 2.
If any CLI step fails: diagnose, fix, and ask the user to re-run that step before continuing.
Expand Down
3 changes: 2 additions & 1 deletion .claude/skills/run_arch.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ Once authorized:
3. `docker compose up -d api worker_light worker_heavy` — recreates containers from fresh images; also clears stale worker memory
4. `docker compose exec api alembic upgrade head` — skip if no SQLAlchemy model changes
5. `docker compose exec api uv run pytest --cov=src --cov-fail-under=90 -q` — full test suite including integration tests against the real DB
6. **Sphinx — run locally, not in Docker** (docs/ is not mounted in the container): `DATABASE_URL="postgresql+psycopg2://dummy:dummy@localhost/dummy" REDIS_URL="redis://localhost:6379/0" uv run sphinx-build -b html docs docs/_build/html -W`"
6. **Sphinx — run locally, not in Docker** (docs/ is not mounted in the container): `DATABASE_URL="postgresql+psycopg2://dummy:dummy@localhost/dummy" REDIS_URL="redis://localhost:6379/0" uv run sphinx-build -b html docs docs/_build/html -W`
7. Commit your changes (`git add` + `git commit`) before starting `genTests` — pre-commit's `--all-files` only checks git-tracked files, so QA validation is unreliable until new files are actually committed."
20 changes: 19 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,30 @@ OTEL_EXPORTER_OTLP_ENDPOINT=http://jaeger:4317
# Production/staging deployment image tag (see doc 24)
IMAGE_TAG=main

# S3-compatible object storage (see doc 27 — local MinIO mock)
# S3-compatible object storage (see doc 27 — local MinIO mock). In a real cloud
# deployment: leave S3_ENDPOINT_URL blank to use AWS's real regional endpoints,
# and leave AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY blank to let boto3's default
# credential chain resolve an IAM role instead of these MinIO dev-only static keys.
S3_ENDPOINT_URL=http://minio:9000
AWS_ACCESS_KEY_ID=minioadmin
AWS_SECRET_ACCESS_KEY=minioadmin
S3_BUCKET_NAME=diffpype-data
S3_REGION=us-east-1
# Host port mappings for the local MinIO container (container S3 API stays on 9000;
# host 9000 is taken by portainer, so the MinIO host API port is 9002)
MINIO_API_PORT=9002
MINIO_CONSOLE_PORT=9001

# Storage backend selection (see doc 28) — "s3" (default, MinIO/S3-compatible) or
# "local" (filesystem-backed, e.g. for a real fiducial FITS set outside MinIO).
# LOCAL_STORAGE_ROOT is the path the app reads/writes *inside* the container
# (fixed at /data via the docker-compose bind mount below) or, for a bare-metal
# run with no Docker at all, a plain relative path on the host.
# LOCAL_STORAGE_HOST_PATH is the *host* directory bind-mounted to /data in
# api/worker_light/worker_heavy — override in .env to point at a real dataset;
# defaults to a git-ignored directory in the repo so a fresh clone works with
# no setup. Put raw FITS files at <LOCAL_STORAGE_HOST_PATH>/raw/ (flat, no
# subdirs — matches the raw/{base_filename} key layout).
STORAGE_BACKEND=s3
LOCAL_STORAGE_ROOT=./data
LOCAL_STORAGE_HOST_PATH=./local_storage_data
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ docs/_build/

# Local data files (Crucial for S3/fits development)
data/
local_storage_data/
*.fits
**/.DS_Store

Expand Down
10 changes: 8 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Claude may only suggest packages or write code that aligns with this strict base
* **Command Authorization Rule:** Claude is explicitly authorized to autonomously execute standard, non-destructive development commands (e.g., `uv sync --all-groups`, `uv lock`, `pytest`, `docker compose build`, `docker compose up`, `docker compose down`) without pausing for human authorization. **Note:** Destructive commands that wipe data volumes (e.g., `docker compose down -v`) strictly require human authorization.
* **Meta-Configuration Change Confirmation:** Before editing this file (`CLAUDE.md`), any file in `.claude/skills/`, `.claude/context/gemini_rules.md`, or `.claude/settings.local.json`, Claude must first propose the change in chat and wait for explicit user confirmation. These files govern how Claude and Gemini behave across every future session, so changes to them are never made silently as a side effect of other work — even when the change seems obviously correct. This does not apply when the user has directly dictated the exact change in the current message (no need to ask them to re-confirm their own instruction).
* **API/CLI Parity:** Any feature, query, or mutation exposed via the FastAPI router must have a corresponding, fully functional command in the `diffpype-manage` CLI. Both boundaries must delegate to the exact same function in the Shared Service Layer.
* **QA/Debugging Fix Confirmation:** When a bug or unexpected failure surfaces during any phase of work — not just `genTests`, this applies universally, including ad-hoc debugging and live incident response — Claude must propose the fix in chat (what's broken, what the fix involves, which layer it belongs in: application code, container/infrastructure config, or both) and wait for explicit confirmation before writing or applying it. When a symptom spans layers, flag that explicitly rather than resolving it unilaterally in just one.
* **Command Aliases:** If the user types `runPrompt on [filename.md]`, immediately read `.claude/skills/run_arch.md` and execute the instructions defined there.

* **Skill Registry:** Claude has the following repeatable skills in `.claude/skills/`. When the user types the trigger command, immediately read the corresponding file and follow it exactly.
Expand All @@ -42,8 +43,9 @@ Claude may only suggest packages or write code that aligns with this strict base
* **Docker State:** If a Celery worker's routing, queue topology, or task signature is changed, Claude must remind the user to run `docker compose restart worker_light worker_heavy` to clear stale memory.
* **Bug Regression Tests:** If Claude fixes a bug, it must proactively write at least one specific unit test designed to catch that exact class of error in the future.
* **Log Everything:** Any bug found and fixed at any phase — during `runPrompt` implementation, during `genTests` QA, or during ad-hoc debugging — must be logged as a dated entry in the `## Logs` section of the relevant architecture MD file immediately after the fix, not batched at the end. The entry must record what the bug was, what triggered its discovery, and what was changed.
* **Pre-Commit Runs Locally, Not in Docker:** `pre-commit` requires a `.git` directory, which is never copied or mounted into the `api`/`worker` containers. Running it via `docker compose exec` fails with `FatalError: git failed`. Always run `uv run pre-commit run --all-files` on the host, exactly like Sphinx.
* **Pre-Commit Runs Locally, Not in Docker:** `pre-commit` requires a `.git` directory, which is never copied or mounted into the `api`/`worker` containers. Running it via `docker compose exec` fails with `FatalError: git failed`. Always run `uv run pre-commit run --all-files` on the host, exactly like Sphinx. **`--all-files` silently skips untracked files** (confirmed empirically during doc 28's `genTests`: every file that later failed CI had been sitting uncommitted the whole session, and every local pre-commit run reported a false "Passed" for them) — this check is only meaningful once new files are actually committed, not just present on disk.
* **Stuck Database States:** When modifying asynchronous tasks, always verify that database rows cannot be left in an orphaned `IN_PROCESS` state if the worker crashes.
* **Streaming & Bounded Resource Use:** Any task processing a batch of external files/objects (downloads, parses, uploads) must do so in a bounded, streaming fashion — one item (or a small fixed chunk) at a time — never materializing an entire batch before doing any work. Peak memory/disk must not scale unboundedly with batch size. Progress must be recoverable/visible (an incrementing counter, not a single all-or-nothing flip), and one item's failure must not take the whole task or its container down with it.
* **Transactions:** Any global or framework-level error handler that intercepts a crash to update a database status MUST explicitly call `db.rollback()` before attempting to write the failure state, preventing `PendingRollbackError` crashes.
* **Database Migrations:** If Claude modifies the SQLAlchemy models in `src/db/models.py`, it must automatically generate the Alembic migration script using `--autogenerate`. Afterward, it must explicitly pause and remind the user to run `alembic upgrade head` (or the equivalent Docker command) to apply the changes to the live database before proceeding.
* **Git Workflow:** The repository uses GitHub Flow. Direct pushes to `main` are forbidden. The feature branch must be created **before** writing the architecture document — the arch doc file itself is a tracked change, and if it is committed on `main` first, it pollutes the PR diff. Claude must remind the user to `git checkout -b feature/[slug]` before Gemini begins drafting and before any implementation code is written. Commit changes on the feature branch and open a Pull Request against `main`.
Expand Down Expand Up @@ -72,4 +74,8 @@ Claude may only suggest packages or write code that aligns with this strict base
* **Action:** Updated the Core Tech Stack Constraints database line to drop the `healpix-alchemy` package name in favor of "native HEALPix range-type spatial columns." Discovered during `assessPrompt` on doc 27 that `healpix-alchemy==1.0.1` (unmaintained since 2021) has an unconstrained `mocpy` version dependency that now resolves to a modern `mocpy` release lacking the `IntervalSet` API its code requires — the package cannot be imported at all as currently pinned. Decided to vendor a small internal `src/db/spatial_types.py` module modeled on its `Tile`/`Point` types against a directly-pinned, working `mocpy` version instead of depending on the broken package. Tracked via GitHub issue #26.
* **Action:** Extended the Model Evaluation Phase to require an effort-level recommendation (low/medium/high/xhigh/max) alongside the model recommendation, and synced the matching wording in `.claude/skills/run_arch.md`. Raised during `runPrompt` on doc 27 — the user wanted effort surfaced as its own axis, not folded into the model choice.
* **Action:** Updated `.claude/skills/gen_tests.md`'s migration-verification step to stop treating an already-applied migration as a failure requiring a database reset. Host and containerized services share one Postgres instance, so a migration Claude already applied and verified live during `runPrompt` will correctly no-op when `genTests` re-runs `alembic upgrade head` — surfaced during `genTests` on doc 27 when this no-op read as a failure. The step now checks `alembic current` first and, when a live demonstration of the migration's code is actually wanted, uses a non-destructive `alembic downgrade -1` / `upgrade head` round-trip (only when the affected tables/columns are confirmed safe to round-trip) instead of a full database reset, which remains a human-authorized last resort only.
* **Action:** Added the CI Service Parity guardrail. Surfaced during `genTests` on doc 27: `minio` was added to `docker-compose.yml` for S3 storage, but `.github/workflows/ci.yml` was never updated to provision it, and this went unnoticed until a real (unmocked) integration test against it was proposed. Distinguished from `redis`, which correctly has no CI service today because nothing in the suite opens a real Redis connection (Celery dispatch is always mocked) — the guardrail's trigger is deliberately "a real integration test needs this service," not "a service exists in `docker-compose.yml`," so it doesn't over-fire on services like `flower`/`portainer`/`jaeger`/`ui` that never need CI wiring.
* **Action:** Added the CI Service Parity guardrail. Surfaced during `genTests` on doc 27: `minio` was added to `docker-compose.yml` for S3 storage, but `.github/workflows/ci.yml` was never updated to provision it, and this went unnoticed until a real (unmocked) integration test against it was proposed. Distinguished from `redis`, which correctly has no CI service today because nothing in the suite opens a real Redis connection (Celery dispatch is always mocked) — the guardrail's trigger is deliberately "a real integration test needs this service," not "a service exists in `docker-compose.yml`," so it doesn't over-fire on services like `flower`/`portainer`/`jaeger`/`ui` that never need CI wiring.

###### 2026-07-28
* **Action:** Amended the Pre-Commit Runs Locally gotcha, and added a commit checkpoint to `.claude/skills/run_arch.md`'s Post-Implementation Verification Checklist plus an explicit, gated pre-commit step to `.claude/skills/gen_tests.md`'s Phase 1. Surfaced after doc 28's PR: CI failed on `ruff`/`ruff-format` even though every local `pre-commit run --all-files` during the entire `genTests` session had reported clean. Root cause, confirmed empirically (a fresh untracked file with a deliberate lint violation got a false "Passed"): `--all-files` only checks git-tracked files, and the whole doc-28 implementation had been sitting uncommitted the entire session — pre-commit silently never checked any of the ~24 new files until the user finally committed and pushed, at which point CI caught them for the first time. Fix is workflow, not automation: the user commits (Claude does not auto-stage or auto-commit, by explicit preference), but `genTests` now checks `git status --short` for untracked entries before trusting a pre-commit run, and `runPrompt`'s checklist now ends with an explicit reminder to commit before `genTests` begins.
* **Action:** Added the QA/Debugging Fix Confirmation guardrail and the Streaming & Bounded Resource Use checklist item. Surfaced during `genTests` on doc 28: `run_ingest_batch` downloaded an entire batch's files into a temp directory before doing any parsing, and a real ~20-file batch of real JWST FITS data OOM-killed the whole `worker_light` container (`WORKER_LIGHT_MEM_LIMIT`/`CELERY_LIGHT_MAX_MEMORY_PER_CHILD` were sized for the trivial Stage-0 dummy `sleep()` task and never revisited). Because an OOM kill (SIGKILL) isn't a catchable Python exception, the task's own crash-handling `except` block never ran, leaving its `IngestBatch` row permanently stuck `in_process` — a direct violation of the existing Stuck Database States guardrail. Claude proposed and applied a streaming per-file fix (download→parse→delete one file at a time, with real incremental progress) without pausing to confirm first, which prompted the QA/Debugging Fix Confirmation addition. Separately, `run_ingest_batch`/`run_mosaic_drizzle` were re-routed from `light` back to `heavy_memory` — real FITS/astropy ingest is memory-hungry even though it's not GPU-bound, and `light`'s multi-child concurrency (`CELERY_LIGHT_CONCURRENCY=4`, all children sharing one container memory ceiling) made it a poor fit regardless of any per-task memory tuning; `heavy` is redefined for now to mean "resource/memory intensive," not GPU-specific — `CeleryQueue.GPU` remains unused/reserved for whenever real GPU-bound work exists (most likely the JWST pipeline doc).
Loading
Loading