Job Traceability, Management, and Audibility Overhaul

.github/instructions/copilot-instructions.md

@@ -140,3 +140,105 @@ poetry run python -m mavedb.scripts.<script_name>
 - [server_main.py](src/mavedb/server_main.py) — App setup and dependency injection
 - [authentication.py](src/mavedb/lib/authentication.py) — Auth patterns
 - [conftest.py](tests/conftest.py) — Test fixtures and database setup
+
+### Naming Conventions
+- **Variables & functions**: `snake_case` (e.g., `score_set_id`, `create_variants_for_score_set`)
+- **Classes**: `PascalCase` (e.g., `ScoreSet`, `UserData`, `ProcessingState`)
+- **Constants**: `UPPER_SNAKE_CASE` (e.g., `MAPPING_QUEUE_NAME`, `DEFAULT_LDH_SUBMISSION_BATCH_SIZE`)
+- **Enum values**: `snake_case` (e.g., `ProcessingState.success`, `MappingState.incomplete`)
+- **Database tables**: `snake_case` with descriptive association table names (e.g., `scoreset_contributors`, `experiment_set_doi_identifiers`)
+- **API endpoints**: kebab-case paths (e.g., `/score-sets`, `/experiment-sets`)
+
+### Documentation Conventions
+*For general Python documentation standards, see `.github/instructions/python.instructions.md`. The following are MaveDB-specific additions:*
+
+- **Algorithm explanations**: Include comments explaining complex logic, especially URN generation and bioinformatics operations
+- **Design decisions**: Comment on why certain architectural choices were made
+- **External dependencies**: Explain purpose of external bioinformatics libraries (HGVS, SeqRepo, etc.)
+- **Bioinformatics context**: Document biological reasoning behind genomic data processing patterns
+
+### Commenting Guidelines
+**Core Principle: Write self-explanatory code. Comment only to explain WHY, not WHAT.**
+
+**✅ WRITE Comments For:**
+- **Complex bioinformatics algorithms**: Variant mapping algorithms, external service interactions
+- **Business logic**: Why specific validation rules exist, regulatory requirements
+- **External API constraints**: Rate limits, data format requirements
+- **Non-obvious calculations**: Score normalization, statistical methods
+- **Configuration values**: Why specific timeouts, batch sizes, or thresholds were chosen
+
+**❌ AVOID Comments For:**
+- **Obvious operations**: Variable assignments, simple loops, basic conditionals
+- **Redundant descriptions**: Comments that repeat what the code clearly shows
+- **Outdated information**: Comments that don't match current implementation
+
+### Error Handling Conventions
+- **Structured logging**: Always use `logger` with `extra=logging_context()` for correlation IDs
+- **HTTP exceptions**: Use FastAPI `HTTPException` with appropriate status codes and descriptive messages
+- **Custom exceptions**: Define domain-specific exceptions in `src/mavedb/lib/exceptions.py`
+- **Worker job errors**: Send Slack notifications via `send_slack_error()` and log with full context
+- **Validation errors**: Use Pydantic validators and raise `ValueError` with clear messages
+
+### Code Style and Organization Conventions
+*For general Python style conventions, see `.github/instructions/python.instructions.md`. The following are MaveDB-specific patterns:*
+
+- **Async patterns**: Use `async def` for I/O operations, regular functions for CPU-bound work
+- **Database operations**: Use SQLAlchemy 2.0 style with `session.scalars(select(...)).one()`
+- **Pydantic models**: Separate request/response models with clear inheritance hierarchies
+- **Bioinformatics data flow**: Structure code to clearly show genomic data transformations
+
+### Testing Conventions
+*For testing philosophy, mocking boundaries, and conventions see `.github/instructions/testing.instructions.md`. For general Python testing standards, see `.github/instructions/python.instructions.md`. The following are MaveDB-specific patterns:*
+
+- **Test function naming**: Use descriptive names that reflect bioinformatics operations (e.g., `test_cannot_publish_score_set_without_variants`)
+- **Fixtures**: Use `conftest.py` for shared fixtures, especially database and worker setup
+- **Mocking**: Mock only at system boundaries (external services, Redis/ARQ, Slack). Do not mock internal helpers or `update_progress`
+- **Constants**: Define test data including genomic sequences and variants in `tests/helpers/constants.py`
+- **Integration testing**: Test full bioinformatics workflows including external service interactions
+
+## Codebase Conventions
+
+### URN Validation
+- Use regex patterns from `src/mavedb/lib/validation/urn_re.py`
+- Validate URNs in Pydantic models with `@field_validator`
+- URN generation logic in `src/mavedb/lib/urns.py` and `temp_urns.py`
+
+### Worker Jobs (ARQ/Redis)
+- **Job definitions**: All background jobs in `src/mavedb/worker/jobs.py`
+- **Settings**: Worker configuration in `src/mavedb/worker/settings.py` with function registry and cron jobs
+- **Job patterns**: 
+  - Use `setup_job_state()` for logging context with correlation IDs
+  - Implement exponential backoff with `enqueue_job_with_backoff()`
+  - Handle database sessions within job context
+  - Send Slack notifications on failures via `send_slack_error()`
+- **Key job types**: 
+  - `create_variants_for_score_set` - Process uploaded CSV data
+  - `map_variants_for_score_set` - External variant mapping via VRS
+  - `submit_score_set_mappings_to_*` - Submit to external annotation services
+- **Enqueueing**: Use `ArqRedis.enqueue_job()` from routers with correlation ID for request tracing
+
+### View Models (Pydantic)
+- **Base model** (`src/mavedb/view_models/base/base.py`) converts empty strings to None and uses camelCase aliases
+- **Inheritance patterns**: `Base` → `Create` → `Modify` → `Saved` model hierarchy
+- **Field validation**: Use `@field_validator` for single fields, `@model_validator(mode="after")` for cross-field validation
+- **URN validation**: Validate URNs with regex patterns from `urn_re.py` in field validators
+- **Transform functions**: Use functions in `validation/transform.py` for complex data transformations
+- **Separate models**: Request (`Create`, `Modify`) vs response (`Saved`) models with different field requirements
+
+### External Integrations
+- **HGVS/SeqRepo** for genomic sequence operations
+- **DCD Mapping** for variant mapping and VRS transformation
+- **CDOT** for transcript/genomic coordinate conversion
+- **GA4GH VRS** for variant representation standardization
+- **ClinGen services** for allele registry and linked data hub submissions
+
+## Key Files to Reference
+- `src/mavedb/models/score_set.py` - Primary data model patterns
+- `src/mavedb/routers/score_sets.py` - Complex router with worker integration
+- `src/mavedb/worker/jobs.py` - Background processing patterns  
+- `src/mavedb/view_models/score_set.py` - Pydantic model hierarchy examples
+- `src/mavedb/server_main.py` - Application setup and dependency injection
+- `src/mavedb/data_providers/services.py` - External service integration patterns
+- `src/mavedb/lib/authentication.py` - Authentication and authorization patterns
+- `tests/conftest.py` - Test fixtures and database setup
+- `docker-compose-dev.yml` - Service architecture and dependencies

.github/instructions/testing.instructions.md

@@ -1,121 +1,88 @@
 ---
-description: 'MaveDB testing conventions — fixtures, mocking, test data patterns'
+description: 'Testing philosophy and conventions for the MaveDB API'
 applyTo: 'tests/**/*.py'
 ---
 
-# Testing Conventions for MaveDB
+# Testing Conventions
 
-## Test Infrastructure
+## Outcome-Based Testing
 
-### Database
-- **pytest-postgresql** provides ephemeral PostgreSQL instances per test session
-- Database schema is created from SQLAlchemy models via `Base.metadata.create_all()`
-- Each test gets a clean transaction that rolls back after completion
-- Core fixtures live in `tests/conftest.py`
+Test what code does (return values, DB state, external boundary calls), not how it does it (internal method calls, message strings, call sequences). Tests should survive internal refactoring without changes.
 
-### Network Isolation
-- **pytest-socket** blocks real network calls in tests
-- External services (HGVS, SeqRepo, DCD Mapping, ClinGen) must be mocked
+**Assert on:**
+- Return values and response objects
+- DB state changes (query for created/updated/deleted records)
+- External boundary calls (see Mocking Boundaries below)
 
-## Fixtures
+**Do not assert on:**
+- Internal function invocations (e.g., that a helper was called with specific args)
+- Call counts or call sequences on internal methods
+- Log or progress message strings
+
+## Mocking Boundaries
+
+Only mock at system boundaries — the edges where your code talks to something external:
+- External services (APIs, third-party clients)
+- Infrastructure (Redis/ARQ, Slack, email)
+- Network I/O (`run_in_executor`, HTTP clients)
+- File I/O (S3, local filesystem in tests)
+
+Do NOT mock internal helpers, validators, or data transforms. Test through them.
+
+## Unit vs Integration Test Responsibilities
+
+**Unit tests:** Edge cases, error paths, invalid inputs, boundary conditions. Use mocked external services.
+
+**Integration tests:** Happy paths, end-to-end workflows, DB state verification. Use real DB with test fixtures.
 
-### Two-Tier conftest
-- `tests/conftest.py` — Core fixtures: database session, auth overrides, user contexts, API client
-- `tests/<module>/conftest.py` — Module-specific fixtures for that test directory
-
-### Auth Fixtures
-Four pre-configured user contexts:
-- **Default user** — standard authenticated user (test ORCID)
-- **Anonymous user** — unauthenticated
-- **Extra user** — second authenticated user (for permission tests)
-- **Admin user** — user with admin role
-
-### DependencyOverrider
-Switch auth context mid-test using the `DependencyOverrider` context manager:
-```python
-with DependencyOverrider(app, {get_current_user: lambda: admin_user}):
-    response = client.get("/api/v1/score-sets/private-urn")
-    assert response.status_code == 200
-```
-
-## Test Data Constants
-
-All test constants live in `tests/helpers/constants.py` with naming conventions:
-
-| Prefix | Purpose | Example |
-|--------|---------|---------|
-| `VALID_*` | Valid input values | `VALID_ACCESSION`, `VALID_GENE_NAME` |
-| `TEST_*` | Complete test objects (dicts) | `TEST_SCORE_SET`, `TEST_EXPERIMENT` |
-| `TEST_MINIMAL_*` | Minimal valid objects | `TEST_MINIMAL_SCORE_SET` |
-| `SAVED_*` | Expected shapes after save | `SAVED_SCORE_SET` |
-| `*_RESPONSE` | Expected API response shapes | `SCORE_SET_RESPONSE` |
+## Assertion Best Practices
+
+- Use `session.refresh()` before asserting on modified ORM objects
+- Add custom assertion messages to complex assertions where the failure message wouldn't immediately clarify what went wrong
+- Include negative assertions where appropriate (verify unwanted records don't exist)
+- Don't add messages to trivially clear assertions like `assert len(variants) == 0`
 
 ## Test Naming
 
-Use descriptive names that reflect the operation and expected outcome:
-```python
-def test_cannot_publish_score_set_without_variants(): ...
-def test_admin_can_view_private_score_set(): ...
-def test_create_experiment_with_invalid_urn_returns_422(): ...
-```
-
-## Mocking External Services
-
-Always mock external bioinformatics services:
-```python
-from unittest.mock import patch
-
-@patch("mavedb.data_providers.services.cdot_rest")
-@patch("mavedb.worker.jobs.map_variants_for_score_set")
-def test_publish_enqueues_mapping(mock_map, mock_cdot, client, db):
-    ...
-```
-
-Common mock targets:
-- `mavedb.data_providers.services.cdot_rest`
-- `mavedb.worker.jobs.*` (individual job functions)
-- `mavedb.lib.authentication.get_current_user`
-- HGVS/SeqRepo data providers
-
-## Helper Factories
-
-Use factory functions in test helpers to create test objects:
-```python
-from tests.helpers.constants import TEST_SCORE_SET
-
-def create_score_set(client, payload=TEST_SCORE_SET):
-    response = client.post("/api/v1/score-sets/", json=payload)
-    assert response.status_code == 201
-    return response.json()
-```
-
-## Testing Patterns
-
-### Permission Testing
-Test both allowed and denied access for each role:
-```python
-def test_owner_can_update_draft(client, db):
-    ...
-
-def test_non_owner_cannot_update_draft(client, db):
-    with DependencyOverrider(app, {get_current_user: lambda: other_user}):
-        response = client.put(f"/api/v1/score-sets/{urn}", json=update_data)
-        assert response.status_code == 404  # 404, not 403
-```
-
-### Worker Job Testing
-Test job logic directly, not through the API:
-```python
-async def test_create_variants_processes_csv(db, score_set):
-    ctx = {"db": db}
-    await create_variants_for_score_set(ctx, score_set.id, "test-correlation-id")
-    assert score_set.num_variants > 0
-```
-
-### Schema Validation
-Verify that response shapes match view models:
-```python
-def test_score_set_response_has_record_type(client):
-    response = client.get(f"/api/v1/score-sets/{urn}")
-    assert response.json()["recordType"] == "score_set"
-```
+Use the pattern: `test_<function_name>_<condition>_<expected_outcome>`
+
+Examples:
+- `test_submit_to_car_when_disabled_skips_submission`
+- `test_create_score_set_returns_422_when_missing_target`
+
+Apply to tests being modified; don't rename all tests at once.
+
+## Parametrization
+
+Use `@pytest.mark.parametrize` with descriptive `ids` when the same logic is tested across multiple states. Prefer parametrization over copy-pasting near-identical tests.
+
+## Fixtures
+
+- Keep fixtures minimal and composable
+- Define fixtures in the most specific `conftest.py` where they're needed
+- Don't duplicate fixtures across test classes — lift shared ones to the nearest common conftest
+- Use factory fixtures when tests need variants of the same object
+
+---
+
+# Worker-Specific Conventions
+
+The following conventions apply specifically to `tests/worker/`.
+
+## Job Test Assertions
+
+- Assert on `JobExecutionOutcome.status` and `.data` for every job test
+- Assert on DB state changes for the domain objects the job modifies
+- For external service jobs: assert boundary calls (ClinGen CAR/LDH, UniProt, gnomAD/Athena, S3, ClinVar)
+
+## Let `update_progress` Run Unpatched
+
+`update_progress()` calls `session.commit()` as a checkpoint. This is production behavior and should execute in tests. Letting it run means tests verify that checkpoint commits don't break state or interfere with final outcomes. Don't patch it, don't mock it, don't assert on its messages.
+
+## TransactionSpy Usage
+
+**USE in manager/decorator tests** (e.g., `test_job_manager.py`, `test_pipeline_manager.py`): The commit/rollback boundary IS the contract here. If someone removes a commit, data silently won't persist in production. DB state checks alone can't catch this because the test session may auto-commit on teardown.
+
+**USE `mock_database_flush_failure` / `mock_database_rollback_failure`**: These simulate DB errors that are genuinely hard to reproduce otherwise. Valuable for testing error recovery paths in infrastructure code.
+
+**DO NOT USE in job-level tests** (e.g., `test_clingen.py`, `test_cleanup.py`, `test_creation.py`): The job's contract is "variants were created" or "stalled jobs were retried," not "session.commit() was called." Use DB state queries instead.

.github/workflows/run-tests-on-push.yml

@@ -1,6 +1,7 @@
-name: Run Tests (On Push)
+name: Run Tests
 on:
   push:
+    # Run all tests on main, fast tests on other branches
 
 env:
   LOG_CONFIG: test
@@ -50,7 +51,12 @@ jobs:
     - run: pip install --upgrade pip
     - run: pip install poetry
     - run: poetry install --with dev
-    - run: poetry run pytest tests/
+    - name: Run fast tests on non-main branches
+      if: github.event_name == 'push' && github.ref != 'refs/heads/main'
+      run: poetry run pytest tests/ -m "not network and not slow"
+    - name: Run full tests on main
+      if: github.event_name == 'push' && github.ref == 'refs/heads/main'
+      run: poetry run pytest tests/
 
   run-tests-3_11:
     runs-on: ubuntu-latest
@@ -66,7 +72,12 @@ jobs:
     - run: pip install --upgrade pip
     - run: pip install poetry
     - run: poetry install --with dev --extras server
-    - run: poetry run pytest tests/ --show-capture=stdout --cov=src
+    - name: Run fast tests on non-main branches
+      if: github.ref != 'refs/heads/main'
+      run: poetry run pytest tests/ -m "not network and not slow" --show-capture=stdout
+    - name: Run all tests with coverage on main branch
+      if: github.ref == 'refs/heads/main'
+      run: poetry run pytest tests/ --show-capture=stdout --cov=src
 
   run-tests-3_12-core-dependencies:
     runs-on: ubuntu-latest
@@ -80,7 +91,12 @@ jobs:
     - run: pip install --upgrade pip
     - run: pip install poetry
     - run: poetry install --with dev
-    - run: poetry run pytest tests/
+    - name: Run fast tests on non-main branches
+      if: github.ref != 'refs/heads/main'
+      run: poetry run pytest tests/ -m "not network and not slow"
+    - name: Run all tests on main branch
+      if: github.ref == 'refs/heads/main'
+      run: poetry run pytest tests/
 
   run-tests-3_12:
     runs-on: ubuntu-latest
@@ -96,4 +112,9 @@ jobs:
     - run: pip install --upgrade pip
     - run: pip install poetry
     - run: poetry install --with dev --extras server
-    - run: poetry run pytest tests/ --show-capture=stdout --cov=src
+    - name: Run fast tests on non-main branches
+      if: github.ref != 'refs/heads/main'
+      run: poetry run pytest tests/ -m "not network and not slow" --show-capture=stdout
+    - name: Run all tests with coverage on main branch
+      if: github.ref == 'refs/heads/main'
+      run: poetry run pytest tests/ --show-capture=stdout --cov=src