gpt-5.3-codex review phase 2 - update instructions & skills
Quality Gate / gate (push) Successful in 34s

This commit is contained in:
Jim Lancaster
2026-08-19 18:22:06 -05:00
parent c261fbb3bd
commit b6a5a89a84
7 changed files with 187 additions and 28 deletions
@@ -0,0 +1,69 @@
---
description: Cross-cutting error handling rules for services, API, and UI.
applyTo: 'src/transcription/**/*.py'
---
# Error Handling (Cross-cutting)
Primary references:
- `docs/ver4/error_handling_v4.md`
- `docs/invariant/error_handling.md`
- `docs/ver4/requirements_v4.md`
## Taxonomy and Categories
Use category-driven semantics aligned to canonical V4 policy:
- `validation`
- `not_found`
- `conflict`
- `external`
- `timeout`
- `internal`
Do not invent ad hoc categories in user/API-facing envelopes unless canonical docs are updated.
## Translation Boundaries
- **Provider/adapters:** raise provider/domain exceptions; do not emit UI text.
- **Services:** map raw exceptions into domain categories and preserve causal chain (`raise ... from ...`).
- **UI/API:** emit user-safe, actionable messages based on category + operation context.
## Retry Rules
- No auto-retry for `validation`, `not_found`, `conflict`.
- `external`/`timeout` may be retried when operation semantics are safe.
- Preserve each retry as new evidence where applicable (no history rewrite).
## Job/Page Failure Semantics
- Page-level (`JobSource`): `pending`, `transcribed`, `failed`, `cancelled`.
- Job terminals: `transcribed`, `partial_success`, `failed`.
- Cancellation must keep job-level and page-level semantics explicit and consistent.
## User-Safe Messaging
- Never leak stack traces, credentials, auth headers, or local filesystem paths in user-facing output.
- Include actionable remediation guidance aligned to category.
- Keep envelope structure consistent across API endpoints.
## Logging and Diagnostics
- Log operation identifiers and error IDs where available.
- Preserve category + cause-chain context.
- Distinguish no-response timeout/network failures from returned provider error responses.
## Guardrails
- No broad catch-and-swallow patterns.
- No success-shaped fallback values after exceptions.
- Category mapping must remain deterministic and testable.
## Contract Sync Rule
If taxonomy, retries, or envelope semantics change:
1. Update canonical docs (`docs/ver4/error_handling_v4.md`, and invariant docs if needed).
2. Update tests in the same change.
3. Update related instruction/skill references.
+13 -22
View File
@@ -20,6 +20,8 @@ applyTo: 'src/transcription/services/*.py'
- Not every module in this package is a service. Helper modules that define no `*Service` - Not every module in this package is a service. Helper modules that define no `*Service`
class (`base`, `errors`, `normalization`, `prompts`, `quality`, `media_storage`, class (`base`, `errors`, `normalization`, `prompts`, `quality`, `media_storage`,
`source_media`) are free-function modules and are exempt from the service rules below. `source_media`) are free-function modules and are exempt from the service rules below.
- Cross-cutting error behavior must follow
[error-handling instructions](./error-handling.instructions.md).
## Model Ownership ## Model Ownership
@@ -69,6 +71,8 @@ module, not in a cross-service import.
which defines no service class and is therefore importable by any of them. which defines no service class and is therefore importable by any of them.
- Use a context manager for large `try/except` blocks, like `handle_transcription_errors` in - Use a context manager for large `try/except` blocks, like `handle_transcription_errors` in
[sources](../../src/transcription/services/sources.py). [sources](../../src/transcription/services/sources.py).
- Category mapping, retry behavior, and translation boundaries are defined in
[error-handling instructions](./error-handling.instructions.md).
## Checklist ## Checklist
@@ -77,6 +81,7 @@ module, not in a cross-service import.
- [ ] Services use `self._session_scope` in their methods to pass the session through - [ ] Services use `self._session_scope` in their methods to pass the session through
- Multiple operations on the same object(s) require sharing a session between all the methods used - Multiple operations on the same object(s) require sharing a session between all the methods used
- [ ] Every model the module touches is either owned by it or reached read-only - [ ] Every model the module touches is either owned by it or reached read-only
- [ ] Evidence writes preserve append-only semantics
## CRUD Methods ## CRUD Methods
@@ -98,18 +103,9 @@ When a service method accepts an optional `session` kwarg, write methods must us
- If `session` is provided: the method must **not** commit; it should `flush()` so IDs and FK values are available to the caller's transaction. - If `session` is provided: the method must **not** commit; it should `flush()` so IDs and FK values are available to the caller's transaction.
- Use `refresh()` on returned ORM objects when the caller needs DB-populated values (defaults, triggers, merged state). - Use `refresh()` on returned ORM objects when the caller needs DB-populated values (defaults, triggers, merged state).
Recommended helper behavior:
- Inputs: active session object, original `session` arg (or a boolean ownership flag), and an optional list of objects to refresh.
- Logic: `commit` when service-owned session, `flush` when caller-owned session, then refresh requested objects.
This keeps orchestration functions atomic: they can pass one shared session across multiple services and commit exactly once at the workflow boundary.
## Workflow Transaction Boundaries ## Workflow Transaction Boundaries
For multi-step job lifecycles (for example queued transcription jobs), orchestration functions must use explicit transaction phases. For multi-step job lifecycles, orchestration functions must use explicit transaction phases.
Required boundary model:
- **Transaction A (claim):** transition `JobStatus.QUEUED -> JobStatus.PROCESSING` and commit immediately. - **Transaction A (claim):** transition `JobStatus.QUEUED -> JobStatus.PROCESSING` and commit immediately.
- Perform provider/network work **outside** database transactions. - Perform provider/network work **outside** database transactions.
@@ -123,28 +119,23 @@ Atomicity rules:
- Terminal state (`TRANSCRIBED` or `FAILED`) and transcript row changes must succeed or roll back together. - Terminal state (`TRANSCRIBED` or `FAILED`) and transcript row changes must succeed or roll back together.
- Retry persistence (`QUEUED` + retry increment + error detail) must succeed or roll back together. - Retry persistence (`QUEUED` + retry increment + error detail) must succeed or roll back together.
Separation of concerns:
- Worker modules should stay lightweight and delegate lifecycle transitions to service/workflow orchestration functions.
- In `workflows.py`, `process_queued_job` should own one complete attempt lifecycle: `QUEUED -> PROCESSING -> TRANSCRIBED|FAILED`.
- In `workflows.py`, `advance_job` should coordinate broader status progression around attempts (for example retry scheduling from `FAILED -> QUEUED`).
- Services should expose session-aware write helpers (flush on caller-owned session) so orchestration controls commit boundaries.
- Backoff/sleep behavior must run outside transactional scopes.
## V4 Contract Alignment ## V4 Contract Alignment
- Treat `docs/ver4/` as the active architecture and requirements baseline. - Treat `docs/ver4/` as the active architecture and requirements baseline.
- Treat `docs/ver4/history.md` and `docs-v4x-archive` as historical-only references.
- `Job.status` success path is `TRANSCRIBED`. - `Job.status` success path is `TRANSCRIBED`.
- `JobSource.status` is queue/projection state only (`PENDING`, `TRANSCRIBED`, `FAILED`, `CANCELLED`). - `JobSource.status` is queue/projection state only (`PENDING`, `TRANSCRIBED`, `FAILED`, `CANCELLED`).
- Source ingest may normalize media before persistence; persisted bytes/hash are canonical for processing and provenance. - Source ingest may normalize media before persistence; persisted bytes/hash are canonical for processing and provenance.
- `ExecutionAttempt` is append-only evidence history; do not mutate historical attempt rows in runtime code.
- `Source.raw_transcription` is a projection, not authoritative history.
- Service/UI read paths that touch relationships must be eager-loaded for `lazy="raise"` compatibility.
- If evidence-related model fields change, update `docs/ver4/schema_v4.md` in the same change.
# Service Composition # Service Composition
A service method may read across models it does not own, using eager loads from its own A service method may read across models it does not own, using eager loads from its own
aggregate root. What it may not do is import another service. aggregate root. What it may not do is import another service.
Operations that must **write** models owned by more than one service — uploading a picture, Operations that must **write** models owned by more than one service are composed in an orchestration module
for example — are composed in an orchestration module
([store](../../src/transcription/services/store.py), ([store](../../src/transcription/services/store.py),
[workflows](../../src/transcription/services/workflows.py)). Orchestration modules define no [workflows](../../src/transcription/services/workflows.py)).
service class, may import any service, and own the commit boundary.
+21 -4
View File
@@ -11,6 +11,9 @@ Keep dependencies flowing in this direction:
Pages may depend on application services and framework-provided dependencies. Components may depend on smaller components and shared presentation helpers. Services and domain modules must never depend on the UI. Pages may depend on application services and framework-provided dependencies. Components may depend on smaller components and shared presentation helpers. Services and domain modules must never depend on the UI.
Cross-cutting error behavior must follow
[error-handling instructions](./error-handling.instructions.md).
## Package Root ## Package Root
- Keep `ui/__init__.py` as the UI composition root: register global assets, register pages, and mount NiceGUI on FastAPI. - Keep `ui/__init__.py` as the UI composition root: register global assets, register pages, and mount NiceGUI on FastAPI.
@@ -44,17 +47,31 @@ Pages may depend on application services and framework-provided dependencies. Co
- Keep all application CSS in `ui/static/theme.css`; do not add page- or component-specific stylesheets or embed style blocks in Python components. - Keep all application CSS in `ui/static/theme.css`; do not add page- or component-specific stylesheets or embed style blocks in Python components.
- Load `theme.css` once from the composition root with `ui.add_css(..., shared=True)`. - Load `theme.css` once from the composition root with `ui.add_css(..., shared=True)`.
- Read stylesheet text through `importlib.resources.files(...)` so loading works from installed packages and is independent of the working directory. - Read stylesheet text through `importlib.resources.files(...)` so loading works from installed packages and is independent of the working directory.
- Centralize CSS reading in one typed helper cached by resource path with `functools.cache` or an equivalent unbounded `lru_cache`. Cache the immutable stylesheet text to avoid repeated resource I/O; keep NiceGUI registration at the composition root. - Centralize CSS reading in one typed helper cached by resource path.
- Do not encode application behavior in CSS or other static assets. - Do not encode application behavior in CSS or other static assets.
## State and Side Effects ## State and Side Effects
- Limit component state to ephemeral interaction state such as loading flags, form values, dialogs, and expansion state. - Limit component state to ephemeral interaction state such as loading flags, form values, dialogs, and expansion state.
- Application and worker state must be resolved at the page or application boundary and passed through narrow interfaces such as callbacks or notifier protocols. - Application and worker state must be resolved at the page or application boundary and passed through narrow interfaces.
- Keep filesystem, network, provider, and worker orchestration behind application services or dedicated adapters. UI code may trigger those operations but must not implement them. - Keep filesystem, network, provider, and worker orchestration behind application services or dedicated adapters.
## Media Route Safety Rules
Two patterns are approved:
1. **Record-validated API routes** for print/export contexts.
2. **Controlled upload URL resolver** (`components/media_urls.py`) for general UI media.
Prohibited patterns:
- Direct `file://` links or exposing local filesystem paths.
- Manual URL construction from raw `Path` values in pages/components.
- User-facing payloads containing local absolute paths.
## V4 Contract Alignment ## V4 Contract Alignment
- Treat `docs/ver4/` as the active baseline and `docs/ver4/history.md` as historical reference only. - Treat `docs/ver4/` as the active baseline and `docs/ver4/history.md` as historical reference only.
- Use status vocabulary exactly as modeled (`queued`, `processing`, `transcribed`, `partial_success`, `failed`; and `pending`, `transcribed`, `failed`, `cancelled`). - Use status vocabulary exactly as modeled (`queued`, `processing`, `transcribed`, `partial_success`, `failed`; and `pending`, `transcribed`, `failed`, `cancelled`).
- Print/export media flows must use record-validated routes from API modules; direct local filesystem paths are prohibited. - Print/export media flows must use record-validated routes; direct local filesystem paths are prohibited.
- If lifecycle wording/behavior changes, update corresponding `docs/ui/pages/*.md` contracts in the same change.
@@ -0,0 +1,79 @@
---
name: evidence-provenance-auditor
description: Deterministic reviewer for transcription evidence/provenance guarantees. Use when changes touch execution attempts, source storage, retries, transport evidence, artifact provenance, or evidence exports.
---
# Evidence & Provenance Auditor
Perform focused, deterministic audits of evidence integrity and provenance behavior.
## When to Use
- Reviewing changes in:
- `src/transcription/services/sources.py`
- `src/transcription/services/store.py`
- `src/transcription/services/workflows.py`
- `src/transcription/services/evidence.py`
- `src/transcription/db/models.py`
- Auditing evidence exports/imports or evidence-display behavior.
- Verifying no drift from canonical provenance invariants.
## Normative References (must be used)
1. `docs/invariant/ai_evidence_and_provenance.md`
2. `docs/ver4/schema_v4.md`
3. `docs/ver4/requirements_v4.md`
4. `docs/ver4/error_handling_v4.md`
5. `docs/ver4/history.md` (archive boundary)
6. `docs-v4x-archive` tag (historical context only)
## Deterministic Pass/Fail Checks
### A. Append-only history
- Every provider call results in a new `ExecutionAttempt`.
- Runtime paths do not mutate historical attempts to represent new outcomes.
- Retry behavior appends attempts rather than rewriting prior rows.
### B. Projection vs authority separation
- `Source.raw_transcription` and preferred pointers are mutable projection surfaces.
- Attempt rows remain authoritative historical evidence.
- Candidate promotion updates projection pointers without rewriting history.
### C. Transport evidence semantics
- Transport evidence is correctly labeled as application-boundary capture.
- SDK snapshots/normalized metadata are not mislabeled as native upstream payload.
- No-response timeout/network states are explicit.
### D. Canonical source identity
- Canonical stored bytes/hash/size are internally consistent.
- If ingest normalization is applied, code/docs consistently represent resulting canonical identity.
- Post-ingest derivatives do not overwrite canonical source bytes.
### E. Secret safety
- No credentials/auth headers/cookies/unrestricted headers persisted.
- Header persistence uses explicit allowlist semantics.
### F. Route/path safety
- Print/export source access is record-validated.
- UI/media path construction does not expose local filesystem paths.
### G. Schema/docs alignment
- Evidence-related model fields and semantics align with canonical docs.
- Evidence model changes require same-change doc updates.
## Review Workflow
1. Read normative references first.
2. Inspect model + service + workflow write paths.
3. Inspect evidence read/display/export paths.
4. Report high-confidence findings with concrete path/line evidence.
5. Classify each finding by invariant family (A-G).
## Output Format
Use this structure:
- Verdict by invariant family (A-G)
- Findings with `Location`, `Observed Behavior`, `Risk`, `Recommended Fix`
- Drift table (`Doc claim` vs `Code reality` vs `Action`)
- Regression guards needed
+4 -1
View File
@@ -39,7 +39,10 @@ When reviewing this repository, always include explicit pass/fail checks for:
2. **UI boundary rule:** pages/components do not perform persistence access (`tests/test_ui_boundaries.py`). 2. **UI boundary rule:** pages/components do not perform persistence access (`tests/test_ui_boundaries.py`).
3. **Status vocabulary conformance:** `JobStatus`/`JobSourceStatus` usage matches current enums in `src/transcription/db/models.py`. 3. **Status vocabulary conformance:** `JobStatus`/`JobSourceStatus` usage matches current enums in `src/transcription/db/models.py`.
4. **Evidence ownership conformance:** append-only attempt history is preserved and projection writes are not mistaken for history mutation (`src/transcription/services/sources.py`, `src/transcription/services/evidence.py`). 4. **Evidence ownership conformance:** append-only attempt history is preserved and projection writes are not mistaken for history mutation (`src/transcription/services/sources.py`, `src/transcription/services/evidence.py`).
5. **Canonical V4 authority:** findings must resolve against `docs/ver4/*` first, and treat `docs/ver4/history.md` plus `docs/ver4.x/*` as historical context. 5. **Canonical V4 authority:** findings must resolve against `docs/ver4/*` first, and treat `docs/ver4/history.md` plus `docs-v4x-archive` as historical context.
6. **Media boundary conformance:** print/export media is record-validated and UI media URL generation uses controlled resolver paths.
7. **Eager-loading conformance:** service/UI read paths satisfy `lazy="raise"` expectations.
8. **Cross-cutting error conformance:** service/API/UI translation and retry behavior align with `.github/instructions/error-handling.instructions.md`.
## Core Review Areas ## Core Review Areas
Binary file not shown.
Binary file not shown.