Skip to content

feat: add RFC 9457 structured error responses (Phase 1)#457

Merged
Aureliolo merged 4 commits intomainfrom
feat/rfc-9457-errors
Mar 15, 2026
Merged

feat: add RFC 9457 structured error responses (Phase 1)#457
Aureliolo merged 4 commits intomainfrom
feat/rfc-9457-errors

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Summary

  • Add machine-readable error metadata to all API error responses following RFC 9457 (Phase 1)
  • ErrorCategory (8-value StrEnum) and ErrorCode (15-value IntEnum) with 4-digit category-grouped codes
  • ErrorDetail frozen Pydantic model (message, error_code, error_category, retryable, retry_after, instance)
  • error_detail field on ApiResponse and PaginatedResponse envelopes (additive, backward-compatible)
  • Class-level error_category/error_code/retryable on ApiError hierarchy with __init_subclass__ validation
  • Request correlation ID binding in RequestLoggingMiddleware via structlog contextvars
  • All 10 exception handlers populate error_detail via centralized _build_error_response()
  • Fix flaky rate limiter pause tests with mocked time.monotonic() (deterministic)

Review coverage

Pre-reviewed by 11 agents (code-reviewer, python-reviewer, test-analyzer, silent-failure-hunter, type-design-analyzer, logging-audit, resilience-audit, conventions-enforcer, security-reviewer, docs-consistency, issue-resolution-verifier). 20 findings addressed across source, tests, and documentation.

Key fixes from review:

  • Security: PERSISTENCE_ERROR code no longer exposed to clients (uses generic INTERNAL_ERROR)
  • Security: Middleware uses clear_correlation_ids() for defensive cleanup
  • Robustness: _get_instance_id() wrapped in try/except (exception handlers must never crash)
  • Type safety: ClassVar annotations + __init_subclass__ enforces code/category consistency
  • Conventions: NotBlankStr on instance, Field(ge=0) on retry_after, redundant overrides removed
  • Tests: Direct unit tests for _get_instance_id, _category_for_status, 5xx scrubbing, ApiError instantiation
  • Docs: CLAUDE.md, docs/design/operations.md, docs/architecture/index.md updated

Test plan

  • uv run ruff check src/ tests/ — lint passes
  • uv run mypy src/ tests/ — type-check passes (975 files)
  • uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80 — 8068 passed, 94.50% coverage
  • uv run pre-commit run --all-files — all hooks pass
  • CI pipeline passes (lint + mypy + pytest + coverage)
  • Verify error_detail appears in error responses via health check or manual test

Closes #419

Add machine-readable error metadata to all API error responses:
- ErrorCategory (8-value StrEnum) and ErrorCode (15-value IntEnum)
  with 4-digit category-grouped codes
- ErrorDetail frozen Pydantic model (message, error_code,
  error_category, retryable, retry_after, instance)
- error_detail field on ApiResponse and PaginatedResponse envelopes
- Class-level error_category/error_code/retryable on ApiError hierarchy
- Request correlation ID binding in RequestLoggingMiddleware
- All 10 exception handlers populate error_detail
- Fix flaky rate limiter pause tests with mocked time

Closes #419
- Add ClassVar annotations + __init_subclass__ validation on ApiError
- Remove redundant retryable=False on non-retryable subclasses
- Use INTERNAL_ERROR (8000) instead of PERSISTENCE_ERROR for client
- Make _get_instance_id defensive (try/except for exception handlers)
- Use clear_correlation_ids() instead of unbind_correlation_id
- Extract _log_request_completion to reduce __call__ below 50 lines
- Add Field(ge=0) on ErrorDetail.retry_after
- Change ErrorDetail.instance to NotBlankStr
- Remove unnecessary int() cast on IntEnum error_code
- Fix bare pytest.raises(Exception) to use ValidationError
- Add direct unit tests for _get_instance_id and _category_for_status
- Add 5xx scrubbing test with custom ServiceUnavailableError message
- Add ApiError instantiation tests (default/custom message, status)
- Add __init_subclass__ mismatch rejection test
- Update CLAUDE.md package structure and logging event constants
- Add RFC 9457 error format docs to operations.md and architecture
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 15, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 67ed576b-0a17-4eb2-86e9-e0aa2e3ad3bf

📥 Commits

Reviewing files that changed from the base of the PR and between 2311c8d and 6006b8a.

📒 Files selected for processing (1)
  • web/src/__tests__/api/client.property.test.ts

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • API responses now include standardized structured error metadata (codes, categories, retry guidance) and per-request correlation IDs.
    • Client libraries surface richer error payloads and throw a dedicated ApiRequestError on failures.
  • Documentation

    • Updated docs with the new structured error format, retry semantics, and stronger logging guidance to use domain-specific event constants.
    • Guidance added: flaky tests must be fixed (use deterministic timing/mocks) rather than skipped.

Walkthrough

Adds RFC 9457 Phase 1 structured error support across API, including ErrorCategory/ErrorCode enums, ErrorDetail DTO, ApiResponse/ PaginatedResponse error_detail fields, exception handler/middleware changes to emit structured errors and correlation IDs, frontend type/client updates, docs, and comprehensive tests.

Changes

Cohort / File(s) Summary
Documentation
CLAUDE.md, docs/architecture/index.md, docs/design/operations.md
Document RFC 9457 Phase 1 error format and logging/event guidance; add guidance about flaky tests and deterministic timing.
Error taxonomy & ApiError subclasses
src/synthorg/api/errors.py
Add ErrorCategory (StrEnum) and ErrorCode (IntEnum), per-class error_category/error_code/retryable ClassVars, and subclass-consistency validation via __init_subclass__. Review new enum values and subclass metadata.
DTOs / Responses
src/synthorg/api/dto.py
Introduce ErrorDetail model and add `error_detail: ErrorDetail
Exception handlers
src/synthorg/api/exception_handlers.py
Refactor handlers to build RFC‑9457-like ErrorDetail payloads, map status→(ErrorCode,ErrorCategory,retryable), propagate correlation instance, and centralize response construction via _build_error_response/_category_for_status.
Middleware / Observability
src/synthorg/api/middleware.py
Bind/generate per-request correlation IDs, add _log_request_completion, log API_REQUEST_COMPLETED/related events, and clear correlation IDs after requests.
Frontend types & client
web/src/api/types.ts, web/src/api/client.ts, web/src/utils/errors.ts
Add ErrorCategory/ErrorCode/ErrorDetail types; extend ApiResponse/PaginatedResponse envelopes with error_detail; add ApiRequestError and helper getErrorDetail + adapt unwrap logic to throw enriched errors.
Tests — DTOs, Errors & Handlers
tests/unit/api/test_dto.py, tests/unit/api/test_dto_properties.py, tests/unit/api/test_errors.py, tests/unit/api/test_exception_handlers.py
Add/extend unit and property tests validating ErrorDetail, enums, ApiError metadata, exception handler mappings (error_detail fields, retryable/retry_after, instance UUIDs), and helper functions.
Tests — Middleware & Resilience
tests/unit/api/test_middleware.py, tests/unit/providers/resilience/test_rate_limiter.py
Add middleware correlation-id lifecycle and logging tests; refactor rate-limiter tests to use mocked monotonic time and asyncio.sleep for deterministic timing.
Tests — Frontend
web/src/__tests__/api/client.property.test.ts
Update mocks/tests to include error_detail in API envelopes and assert discriminated-union behavior.
Manifest
package.json
Minor manifest update referenced by frontend changes.

Sequence Diagram

sequenceDiagram
    autonumber
    actor Client
    participant Middleware as Middleware (Correlation)
    participant Handler as AppHandler
    participant Exception as ExceptionHandler
    participant DTO as ErrorDetailModel
    participant Response as ApiResponse

    Client->>Middleware: send HTTP request
    Middleware->>Middleware: generate_correlation_id()
    Middleware->>Middleware: bind_correlation_id(id)
    Middleware->>Handler: forward request

    alt handler succeeds
        Handler->>Response: return ApiResponse(data, error=null, error_detail=null)
    else handler raises
        Handler->>Exception: raise ApiError / HTTP exception
        Exception->>Exception: _category_for_status(status)
        Exception->>Middleware: _get_instance_id() (correlation id)
        Exception->>DTO: construct ErrorDetail(message, error_code, error_category, retryable, retry_after, instance)
        DTO->>Exception: ErrorDetail
        Exception->>Response: _build_error_response(with error_detail)
    end

    Response->>Middleware: send response
    Middleware->>Middleware: _log_request_completion(status)
    Middleware->>Middleware: clear_correlation_ids()
    Response->>Client: ApiResponse (+ error_detail when error)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and concisely summarizes the main change: adding RFC 9457 structured error responses, which is the primary feature across the entire changeset.
Description check ✅ Passed Description is comprehensive and directly related to the changeset, providing context on RFC 9457 implementation, key changes, review coverage, test results, and closes a linked issue.
Linked Issues check ✅ Passed All core Phase 1 objectives from #419 are implemented: ErrorCategory enum, ErrorCode enum, ErrorDetail model with required fields (retryable, retry_after, error_code, error_category, instance), error_detail added to response envelopes, ApiError hierarchy updated with metadata, exception handlers populate structured errors.
Out of Scope Changes check ✅ Passed All changes are tightly scoped to RFC 9457 Phase 1 implementation. The rate limiter test mocking and middleware correlation ID binding are closely related enablers. Documentation updates and frontend type sync are necessary supporting changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/rfc-9457-errors
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/rfc-9457-errors
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly upgrades the API's error reporting capabilities by introducing structured, machine-readable error responses based on RFC 9457. This change provides clients with detailed error context, including standardized codes, categories, and retry information, which is crucial for building more resilient and intelligent client applications. Additionally, it improves the overall stability of the system by addressing test flakiness and enhancing logging with robust request correlation IDs.

Highlights

  • RFC 9457 Structured Error Responses: Implemented machine-readable error metadata for all API error responses, adhering to RFC 9457 (Phase 1). This includes new ErrorCategory (8-value StrEnum) and ErrorCode (15-value IntEnum) with category-grouped codes.
  • Error Detail Model: Introduced a ErrorDetail frozen Pydantic model containing message, error_code, error_category, retryable, retry_after, and instance (request correlation ID).
  • API Response Integration: Added the error_detail field to ApiResponse and PaginatedResponse envelopes, ensuring backward compatibility.
  • Error Hierarchy Enhancements: Enhanced the ApiError hierarchy with class-level error_category, error_code, and retryable attributes, validated via __init_subclass__ for consistency.
  • Request Correlation IDs: Implemented request correlation ID binding in RequestLoggingMiddleware using structlog contextvars, and ensured defensive cleanup with clear_correlation_ids().
  • Centralized Error Handling: All 10 existing exception handlers now populate error_detail through a new centralized _build_error_response() function.
  • Flaky Test Fixes: Resolved flaky rate limiter pause tests by mocking time.monotonic() and asyncio.sleep() to ensure deterministic behavior.
  • Security Improvements: The PERSISTENCE_ERROR code is no longer exposed to clients, instead using a generic INTERNAL_ERROR for enhanced security.
  • Robustness: The _get_instance_id() helper function is now wrapped in a try/except block to prevent crashes within exception handlers.
Changelog
  • CLAUDE.md
    • Updated the API description to include RFC 9457 structured errors.
    • Added a new guideline for fixing flaky tests by mocking timing functions.
  • docs/architecture/index.md
    • Updated the API module description to reflect the inclusion of RFC 9457 structured error responses.
  • docs/design/operations.md
    • Added a new section detailing the RFC 9457 error response format, including a table of fields and their descriptions.
  • src/synthorg/api/dto.py
    • Imported ErrorCategory for use in the new error detail model.
    • Defined the ErrorDetail Pydantic model for structured error metadata.
    • Added an error_detail field to ApiResponse and PaginatedResponse models.
  • src/synthorg/api/errors.py
    • Introduced ErrorCategory (StrEnum) and ErrorCode (IntEnum) for structured error classification.
    • Added error_category, error_code, and retryable class variables to ApiError and its subclasses.
    • Implemented __init_subclass__ validation to ensure consistency between error_code prefixes and error_category.
    • Updated docstrings to reflect the new error taxonomy.
  • src/synthorg/api/exception_handlers.py
    • Imported structlog, ErrorDetail, ErrorCategory, ErrorCode, and generate_correlation_id.
    • Added _get_instance_id() to retrieve or defensively generate a request correlation ID.
    • Created _build_error_response() to construct ApiResponse objects with structured ErrorDetail.
    • Defined mappings (_STATUS_TO_ERROR_META, _CLIENT_ERROR_DEFAULT, _SERVER_ERROR_DEFAULT) for HTTP status codes to error metadata.
    • Implemented _category_for_status() to map HTTP status codes to error code, category, and retryability.
    • Modified all existing exception handlers to utilize _build_error_response() and populate the error_detail field.
    • Updated handle_persistence_error to return a generic 'Internal server error' message and INTERNAL_ERROR code for security reasons.
  • src/synthorg/api/middleware.py
    • Imported correlation ID utilities (bind_correlation_id, clear_correlation_ids, generate_correlation_id).
    • Refactored request completion logging into a _log_request_completion helper function.
    • Modified RequestLoggingMiddleware.__call__ to bind a new correlation ID at the start of a request and clear it upon completion.
  • tests/unit/api/test_dto.py
    • Imported ErrorDetail, PaginatedResponse, PaginationMeta, and ErrorCategory.
    • Added tests for ApiResponse and PaginatedResponse to verify error_detail is None on success and populated on error.
    • Added a new test class TestErrorDetail to cover ErrorDetail construction, default values, immutability, and serialization.
  • tests/unit/api/test_dto_properties.py
    • Imported ErrorDetail and ErrorCategory.
    • Added a Hypothesis strategy _error_detail_strategy for generating ErrorDetail instances.
    • Added TestErrorDetailProperties class with property-based tests for ErrorDetail roundtrip serialization and preservation of retryable and error_category.
  • tests/unit/api/test_errors.py
    • Added a new file containing comprehensive unit tests for ErrorCategory and ErrorCode enums, including completeness, values, and consistency checks.
    • Tested the ApiError hierarchy for class-level metadata (error_category, error_code, retryable) and __init_subclass__ validation.
    • Included tests for the instantiation behavior of ApiError subclasses.
  • tests/unit/api/test_exception_handlers.py
    • Imported new error types and the re module for UUID validation.
    • Added a _UUID_RE regex for validating UUID formats.
    • Created an _assert_error_detail helper function to validate the structure of error details in responses.
    • Updated all existing exception handler tests to assert the presence and correctness of the error_detail field.
    • Added a new test class TestStructuredErrorMetadata for specific RFC 9457 features, including retryable errors, UUID format for instance, message matching, and 5xx scrubbing.
    • Added a new test class TestGetInstanceId for direct unit testing of the _get_instance_id helper, covering context retrieval and fallback UUID generation.
    • Added a new test class TestCategoryForStatus for direct unit testing of the _category_for_status helper, covering mapped and unmapped HTTP status codes.
    • Added TestApiErrorInstantiation for ApiError and subclass instantiation behavior.
  • tests/unit/providers/resilience/test_rate_limiter.py
    • Removed the direct import time statement.
    • Modified TestRateLimiterPause tests to use unittest.mock.patch for time.monotonic and asyncio.sleep, making them deterministic and resolving flakiness.
Activity
  • Pre-reviewed by 11 agents (code-reviewer, python-reviewer, test-analyzer, silent-failure-hunter, type-design-analyzer, logging-audit, resilience-audit, conventions-enforcer, security-reviewer, docs-consistency, issue-resolution-verifier).
  • 20 findings addressed across source, tests, and documentation.
  • Security: PERSISTENCE_ERROR code no longer exposed to clients (uses generic INTERNAL_ERROR).
  • Security: Middleware uses clear_correlation_ids() for defensive cleanup.
  • Robustness: _get_instance_id() wrapped in try/except.
  • Type safety: ClassVar annotations + __init_subclass__ enforces code/category consistency.
  • Conventions: NotBlankStr on instance, Field(ge=0) on retry_after, redundant overrides removed.
  • Tests: Direct unit tests for _get_instance_id, _category_for_status, 5xx scrubbing, ApiError instantiation.
  • Docs: CLAUDE.md, docs/design/operations.md, docs/architecture/index.md updated.
  • Test plan includes lint, type-check, pytest with coverage, pre-commit hooks, CI pipeline, and manual verification of error_detail.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 18:25 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a robust and well-designed structured error reporting mechanism following RFC 9457. The changes are comprehensive and of high quality, including a new error taxonomy, updated DTOs and exception handlers, and improved middleware for correlation IDs. The use of __init_subclass__ for validation and the addition of thorough tests are particularly noteworthy. My only suggestion is to consider expanding the ApiError hierarchy to map more domain-specific exceptions to this new structured format, which would further enhance the utility of this excellent feature.

}


class ApiError(Exception):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This new error taxonomy is a great foundation. To fully leverage it, consider introducing more specific ApiError subclasses for other key domain errors. For example, domain exceptions like BudgetExhaustedError or provider-related errors could be caught at the API boundary and re-raised as new ApiBudgetExhaustedError(ApiError) or ApiProviderError(ApiError) subclasses.

This would allow you to return more specific error codes (e.g., BUDGET_EXHAUSTED, PROVIDER_ERROR) and categories to the client, rather than having them fall through to the generic INTERNAL_ERROR. This would make the API more informative for consumers.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 95.31250% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.69%. Comparing base (f60746a) to head (6006b8a).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/synthorg/api/dto.py 89.28% 2 Missing and 1 partial ⚠️
src/synthorg/api/middleware.py 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
+ Coverage   93.67%   93.69%   +0.01%     
==========================================
  Files         469      469              
  Lines       22219    22331     +112     
  Branches     2143     2152       +9     
==========================================
+ Hits        20814    20922     +108     
- Misses       1095     1098       +3     
- Partials      310      311       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Line 190: The long inline list of event constants (e.g., PROVIDER_CALL_START,
BUDGET_RECORD_ADDED, CFO_ANOMALY_DETECTED, MEETING_STARTED,
CLASSIFICATION_START, MEMORY_RETRIEVAL_START, TASK_CREATED,
APPROVAL_GATE_ESCALATION_DETECTED, etc.) hurts readability; condense this by
replacing the exhaustive list with a short explanatory sentence pointing to
synthorg.observability.events and an example import pattern (e.g., "from
synthorg.observability.events.<domain> import EVENT_CONSTANT"), or move the full
list into a separate reference document and link to it from the guideline;
update the CLAUDE.md section that currently enumerates the constants so it
contains only the module reference and an example, and add the full enumeration
to a referenced doc for maintainability.

In `@tests/unit/providers/resilience/test_rate_limiter.py`:
- Line 100: Move the repeated local import "from unittest import mock" to the
module-level imports and delete the in-function imports found inside the test
methods (the occurrences currently inside the test functions at the five
locations where "from unittest import mock" is repeated); update the top of the
test module to include "from unittest import mock" and remove the redundant
lines from each test so all tests use the single module-level mock import.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 74366d95-bf34-47d6-a6bb-11bada2bf271

📥 Commits

Reviewing files that changed from the base of the PR and between f60746a and 80341f5.

📒 Files selected for processing (12)
  • CLAUDE.md
  • docs/architecture/index.md
  • docs/design/operations.md
  • src/synthorg/api/dto.py
  • src/synthorg/api/errors.py
  • src/synthorg/api/exception_handlers.py
  • src/synthorg/api/middleware.py
  • tests/unit/api/test_dto.py
  • tests/unit/api/test_dto_properties.py
  • tests/unit/api/test_errors.py
  • tests/unit/api/test_exception_handlers.py
  • tests/unit/providers/resilience/test_rate_limiter.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Build Backend
  • GitHub Check: Build Web
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

All docstrings in public APIs must be documented in Google style and reflected in docs/api/ auto-generated library reference (via mkdocstrings + Griffe AST-based parsing)

Files:

  • docs/architecture/index.md
  • docs/design/operations.md
docs/design/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS read the relevant design page before implementing any feature or planning any issue. If implementation deviates from the spec, alert the user and explain why — user decides whether to proceed or update the spec

Files:

  • docs/design/operations.md
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations
Use except A, B: syntax (no parentheses) instead of except (A, B): — PEP 758 syntax enforced by ruff on Python 3.14
All public functions must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Use NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Use @computed_field in Pydantic models for derived values instead of storing and validating redundant fields
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) instead of bare create_task
Line length: 88 characters (enforced by ruff)
Functions must be < 50 lines; files must be < 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate input at system boundaries (user input, external APIs, config files)
Create new objects instead of mutating existing ones (immutability principle)

Files:

  • tests/unit/api/test_errors.py
  • tests/unit/providers/resilience/test_rate_limiter.py
  • tests/unit/api/test_dto_properties.py
  • tests/unit/api/test_exception_handlers.py
  • src/synthorg/api/exception_handlers.py
  • tests/unit/api/test_dto.py
  • src/synthorg/api/errors.py
  • src/synthorg/api/dto.py
  • src/synthorg/api/middleware.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, @pytest.mark.slow to categorize tests
Maintain 80% minimum code coverage (enforced in CI)
Each test must complete within 30 seconds (timeout enforcement)
Always include -n auto when running pytest via uv run python -m pytest — never run tests sequentially (pytest-xdist parallelism)
Prefer @pytest.mark.parametrize for testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like example-provider, example-large-001, test-provider, test-small-001, or size aliases (large/medium/small)
Use Hypothesis for property-based testing with @given + @settings decorators; control profiles via HYPOTHESIS_PROFILE env var (ci for 200 examples, dev for 1000 examples)

Files:

  • tests/unit/api/test_errors.py
  • tests/unit/providers/resilience/test_rate_limiter.py
  • tests/unit/api/test_dto_properties.py
  • tests/unit/api/test_exception_handlers.py
  • tests/unit/api/test_dto.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every module with business logic must import from synthorg.observability import get_logger and define logger = get_logger(__name__)
Never use import logging, logging.getLogger(), or print() in application code — use the synthorg logger instead
Always use event name constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider); import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always log with structured kwargs: logger.info(EVENT, key=value) — never use old-style formatting logger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Never implement retry logic in driver subclasses or calling code — all provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically
Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig; retryable errors are RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError
Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state
For dict/list fields in frozen Pydantic models, use copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use Pydantic v2 conventions: BaseModel, model_validator, computed_field, ConfigDict

Files:

  • src/synthorg/api/exception_handlers.py
  • src/synthorg/api/errors.py
  • src/synthorg/api/dto.py
  • src/synthorg/api/middleware.py
🧠 Learnings (22)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : Rate limiter respects RateLimitError.retry_after from providers — automatically pauses future requests.

Applied to files:

  • tests/unit/providers/resilience/test_rate_limiter.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Always log with structured kwargs: `logger.info(EVENT, key=value)` — never use old-style formatting `logger.info("msg %s", val)`

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic must import `from synthorg.observability import get_logger` and define `logger = get_logger(__name__)`

Applied to files:

  • CLAUDE.md
  • src/synthorg/api/middleware.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.

Applied to files:

  • CLAUDE.md
  • src/synthorg/api/middleware.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging`, `logging.getLogger()`, or `print()` in application code — use the synthorg logger instead

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to tests/**/*.py : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like `example-provider`, `example-large-001`, `test-provider`, `test-small-001`, or size aliases (`large`/`medium`/`small`)

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to {src,tests,web,cli,site}/**/*.{py,ts,tsx,go,astro} : Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2) .claude/ skill/agent files, (3) third-party import paths/module names.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Parallelism: pytest-xdist via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to cli/**/*.go : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, or config examples — use generic names

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to web/**/*.{ts,tsx,js,jsx,vue} : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, or config examples — use generic names like `example-provider`, `example-large-001`

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`

Applied to files:

  • src/synthorg/api/dto.py
🧬 Code graph analysis (6)
tests/unit/api/test_errors.py (1)
src/synthorg/api/errors.py (9)
  • ApiError (86-117)
  • ApiValidationError (131-139)
  • ConflictError (142-150)
  • ErrorCategory (14-27)
  • ErrorCode (30-68)
  • ForbiddenError (153-161)
  • NotFoundError (120-128)
  • ServiceUnavailableError (175-184)
  • UnauthorizedError (164-172)
tests/unit/api/test_dto_properties.py (2)
src/synthorg/api/dto.py (1)
  • ErrorDetail (40-63)
src/synthorg/api/errors.py (1)
  • ErrorCategory (14-27)
tests/unit/api/test_exception_handlers.py (2)
src/synthorg/api/errors.py (8)
  • ApiError (86-117)
  • ApiValidationError (131-139)
  • ConflictError (142-150)
  • ErrorCategory (14-27)
  • ErrorCode (30-68)
  • ForbiddenError (153-161)
  • ServiceUnavailableError (175-184)
  • UnauthorizedError (164-172)
src/synthorg/api/exception_handlers.py (3)
  • handle_unexpected (212-225)
  • _get_instance_id (45-59)
  • _category_for_status (106-114)
src/synthorg/api/exception_handlers.py (2)
src/synthorg/api/dto.py (2)
  • ApiResponse (69-89)
  • ErrorDetail (40-63)
src/synthorg/api/errors.py (3)
  • ApiError (86-117)
  • ErrorCategory (14-27)
  • ErrorCode (30-68)
tests/unit/api/test_dto.py (2)
src/synthorg/api/dto.py (6)
  • ErrorDetail (40-63)
  • PaginatedResponse (108-130)
  • PaginationMeta (92-105)
  • ApiResponse (69-89)
  • success (87-89)
  • success (128-130)
src/synthorg/api/errors.py (1)
  • ErrorCategory (14-27)
src/synthorg/api/dto.py (1)
src/synthorg/api/errors.py (1)
  • ErrorCategory (14-27)
🔇 Additional comments (27)
src/synthorg/api/middleware.py (1)

25-29: LGTM! Clean correlation ID lifecycle management.

The correlation ID is properly bound at request start and defensively cleared in a finally block, ensuring cleanup even on exceptions. The _log_request_completion helper appropriately uses WARNING level when status is unknown (indicating an incomplete response flow).

Also applies to: 173-173, 193-194

src/synthorg/api/errors.py (2)

106-117: Excellent use of __init_subclass__ for compile-time validation.

This ensures error code/category consistency is validated at class definition time rather than runtime, catching misconfigurations early. The validation logic correctly extracts the category prefix and fails fast with a clear TypeError.


14-83: LGTM! Well-structured error taxonomy.

The error codes are logically grouped by category prefix (1xxx-8xxx), and the _CODE_CATEGORY_PREFIX mapping enables runtime validation. The enum values align with the RFC 9457 Phase 1 requirements.

src/synthorg/api/dto.py (2)

40-63: LGTM! Well-designed ErrorDetail model.

The model correctly uses frozen=True for immutability, NotBlankStr for the correlation ID instance field, and ge=0 constraint on retry_after. Using int for error_code (rather than ErrorCode enum) provides flexibility for edge cases while keeping the model self-contained.


83-83: LGTM! Backward-compatible envelope changes.

Adding error_detail as an optional field with None default ensures existing clients are unaffected. The field is present on both ApiResponse and PaginatedResponse for consistency.

Also applies to: 123-123

src/synthorg/api/exception_handlers.py (3)

45-59: Defensive error handling is appropriate here.

The silent except with pass at lines 56-58 is intentionally documented ("Exception handlers must never crash — silent fallback is intentional"). This is the correct pattern for code running inside exception handlers, which are the last line of defense. The fallback to generate_correlation_id() ensures a valid instance ID is always returned.


62-114: LGTM! Clean centralized error response construction.

The _build_error_response helper and status-to-metadata mapping provide a single source of truth for structured error responses. The retryable flags are correctly set (429, 503 → True), and the fallback logic appropriately defaults to validation for 4xx and internal for 5xx.


188-209: LGTM! ApiError handler correctly extracts class-level metadata.

The handler properly accesses the class-level error_code, error_category, and retryable attributes from the exception type, preserving the 5xx message scrubbing pattern (line 198 uses type(exc).default_message for server errors).

docs/architecture/index.md (1)

40-40: LGTM! Documentation accurately reflects the new feature.

The addition of "RFC 9457 structured error responses" to the API module description is concise and accurate.

tests/unit/api/test_dto_properties.py (1)

223-263: LGTM! Solid property-based tests for ErrorDetail.

The strategy correctly generates valid instances with appropriate constraints (error codes 1000-8999, non-blank instance via filter). The roundtrip test validates serialization integrity, and the field preservation test confirms retryable/category are maintained.

docs/design/operations.md (1)

978-996: LGTM! Comprehensive documentation for the new error format.

The section clearly documents all error_detail fields, error code groupings, and provides actionable guidance for agent consumers on using retryable/retry_after for autonomous retry logic. This aligns well with the RFC 9457 Phase 1 objectives.

CLAUDE.md (2)

115-115: LGTM! Accurate package structure update.

The API module description now correctly reflects the RFC 9457 structured error additions.


217-217: LGTM! Valuable guidance on flaky test remediation.

The directive to fix flaky tests with deterministic mocks (rather than widening timing margins or skipping) is a best practice that improves test reliability.

tests/unit/providers/resilience/test_rate_limiter.py (2)

340-353: LGTM!

The logging tests correctly verify pause and throttle event emission. The use of structlog.testing.capture_logs() is appropriate for validating log output.


98-139: LGTM!

The refactored pause tests use well-structured mocking to achieve determinism. The time_fn closure correctly simulates time progression through the pause window, and fake_sleep captures the sleep duration for assertions.

tests/unit/api/test_dto.py (3)

45-64: LGTM!

The tests correctly verify that error_detail is None on success and properly populated on error. The assertions cover both object attributes and serialized output.


66-116: LGTM!

Comprehensive coverage of ErrorDetail construction, defaults, immutability, and serialization. The frozen model behavior is correctly validated with pytest.raises(ValidationError).


118-140: LGTM!

Tests correctly verify error_detail defaults to None and is properly accessible when provided on PaginatedResponse.

tests/unit/api/test_errors.py (3)

1-18: LGTM!

Well-structured test module for the RFC 9457 error taxonomy. The imports cover all error classes and the pytestmark correctly applies @pytest.mark.unit to all tests.


39-86: LGTM!

Excellent use of @pytest.mark.parametrize for testing enum members. The prefix consistency test (lines 79-85) ensures all error codes follow the category-grouped convention documented in the enum.


139-145: Verify that the test class definition is complete.

The _BadError class definition appears to end at line 145 without a pass statement or method body. In Python, a class body requires at least one statement. However, since the error_code assignment is present, this should work correctly. The test relies on the TypeError being raised during class creation (in __init_subclass__), which happens before the class body would need additional statements.

tests/unit/api/test_exception_handlers.py (6)

34-36: LGTM!

The _UUID_RE pattern correctly validates standard UUID format, and _assert_error_detail is an excellent helper that reduces duplication across all exception handler tests while ensuring consistent validation of the RFC 9457 error structure.

Also applies to: 46-64


67-455: LGTM!

Comprehensive updates to all exception handler tests with proper _assert_error_detail validation. The tests correctly verify:

  • Error code and category mappings for each exception type
  • Retryable flags (True for 429, False for most others)
  • 5xx message scrubbing (internal details not exposed)

551-566: Good security test coverage.

The test_5xx_scrubs_custom_message test correctly verifies that internal details (like IP addresses) are not leaked in 5xx error responses. This is critical for preventing information disclosure.


568-615: LGTM!

Thorough unit tests for _get_instance_id covering:

  • Happy path (context has valid request_id)
  • Fallback when no context
  • Edge cases (non-string, empty string)

The try/finally cleanup pattern correctly prevents test pollution.


618-691: LGTM!

Excellent parametrized tests for _category_for_status covering all mapped statuses and both unmapped server/client error fallbacks. The TestApiErrorInstantiation class correctly validates default messages, status codes, and custom message precedence.


1-3: LGTM!

The updated docstring accurately reflects the test file's expanded scope to cover RFC 9457 structured error responses.

CLAUDE.md Outdated
- **Never** use `import logging` / `logging.getLogger()` / `print()` in application code
- **Variable name**: always `logger` (not `_logger`, not `log`)
- **Event names**: always use constants from the domain-specific module under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, `CFO_ANOMALY_DETECTED` from `events.cfo`, `CONFLICT_DETECTED` from `events.conflict`, `MEETING_STARTED` from `events.meeting`, `MEETING_SCHEDULER_STARTED` from `events.meeting`, `MEETING_SCHEDULER_ERROR` from `events.meeting`, `MEETING_SCHEDULER_STOPPED` from `events.meeting`, `MEETING_PERIODIC_TRIGGERED` from `events.meeting`, `MEETING_EVENT_TRIGGERED` from `events.meeting`, `MEETING_PARTICIPANTS_RESOLVED` from `events.meeting`, `MEETING_NO_PARTICIPANTS` from `events.meeting`, `MEETING_NOT_FOUND` from `events.meeting`, `CLASSIFICATION_START` from `events.classification`, `CONSOLIDATION_START` from `events.consolidation`, `ORG_MEMORY_QUERY_START` from `events.org_memory`, `API_REQUEST_STARTED` from `events.api`, `API_ROUTE_NOT_FOUND` from `events.api`, `API_COORDINATION_STARTED` from `events.api`, `API_COORDINATION_COMPLETED` from `events.api`, `API_COORDINATION_FAILED` from `events.api`, `API_COORDINATION_AGENT_RESOLVE_FAILED` from `events.api`, `CODE_RUNNER_EXECUTE_START` from `events.code_runner`, `DOCKER_EXECUTE_START` from `events.docker`, `MCP_INVOKE_START` from `events.mcp`, `SECURITY_EVALUATE_START` from `events.security`, `HR_HIRING_REQUEST_CREATED` from `events.hr`, `PERF_METRIC_RECORDED` from `events.performance`, `TRUST_EVALUATE_START` from `events.trust`, `PROMOTION_EVALUATE_START` from `events.promotion`, `PROMPT_BUILD_START` from `events.prompt`, `MEMORY_RETRIEVAL_START` from `events.memory`, `MEMORY_BACKEND_CONNECTED` from `events.memory`, `MEMORY_ENTRY_STORED` from `events.memory`, `MEMORY_BACKEND_SYSTEM_ERROR` from `events.memory`, `AUTONOMY_ACTION_AUTO_APPROVED` from `events.autonomy`, `TIMEOUT_POLICY_EVALUATED` from `events.timeout`, `PERSISTENCE_AUDIT_ENTRY_SAVED` from `events.persistence`, `TASK_ENGINE_STARTED` from `events.task_engine`, `COORDINATION_STARTED` from `events.coordination`, `COORDINATION_FACTORY_BUILT` from `events.coordination`, `COMMUNICATION_DISPATCH_START` from `events.communication`, `COMPANY_STARTED` from `events.company`, `CONFIG_LOADED` from `events.config`, `CORRELATION_ID_CREATED` from `events.correlation`, `DECOMPOSITION_STARTED` from `events.decomposition`, `DELEGATION_STARTED` from `events.delegation`, `EXECUTION_LOOP_START` from `events.execution`, `CHECKPOINT_SAVED` from `events.checkpoint`, `PERSISTENCE_CHECKPOINT_SAVED` from `events.persistence`, `GIT_OPERATION_START` from `events.git`, `PARALLEL_GROUP_START` from `events.parallel`, `PERSONALITY_LOADED` from `events.personality`, `QUOTA_CHECKED` from `events.quota`, `ROLE_ASSIGNED` from `events.role`, `ROUTING_STARTED` from `events.routing`, `SANDBOX_EXECUTE_START` from `events.sandbox`, `TASK_CREATED` from `events.task`, `TASK_ASSIGNMENT_STARTED` from `events.task_assignment`, `TASK_ROUTING_STARTED` from `events.task_routing`, `TEMPLATE_LOADED` from `events.template`, `TOOL_INVOKE_START` from `events.tool`, `TOOL_OUTPUT_WITHHELD` from `events.tool`, `WORKSPACE_CREATED` from `events.workspace`, `APPROVAL_GATE_ESCALATION_DETECTED` from `events.approval_gate`, `APPROVAL_GATE_ESCALATION_FAILED` from `events.approval_gate`, `APPROVAL_GATE_INITIALIZED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFIED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFY_FAILED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARKED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARK_FAILED` from `events.approval_gate`, `APPROVAL_GATE_PARK_TASKLESS` from `events.approval_gate`, `APPROVAL_GATE_RESUME_STARTED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_RESUMED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_DELETE_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_TRIGGERED` from `events.approval_gate`, `APPROVAL_GATE_NO_PARKED_CONTEXT` from `events.approval_gate`, `APPROVAL_GATE_LOOP_WIRING_WARNING` from `events.approval_gate`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
- **Event names**: always use constants from the domain-specific module under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, `CFO_ANOMALY_DETECTED` from `events.cfo`, `CONFLICT_DETECTED` from `events.conflict`, `MEETING_STARTED` from `events.meeting`, `MEETING_SCHEDULER_STARTED` from `events.meeting`, `MEETING_SCHEDULER_ERROR` from `events.meeting`, `MEETING_SCHEDULER_STOPPED` from `events.meeting`, `MEETING_PERIODIC_TRIGGERED` from `events.meeting`, `MEETING_EVENT_TRIGGERED` from `events.meeting`, `MEETING_PARTICIPANTS_RESOLVED` from `events.meeting`, `MEETING_NO_PARTICIPANTS` from `events.meeting`, `MEETING_NOT_FOUND` from `events.meeting`, `CLASSIFICATION_START` from `events.classification`, `CONSOLIDATION_START` from `events.consolidation`, `ORG_MEMORY_QUERY_START` from `events.org_memory`, `API_REQUEST_STARTED` from `events.api`, `API_REQUEST_ERROR` from `events.api`, `API_ROUTE_NOT_FOUND` from `events.api`, `API_COORDINATION_STARTED` from `events.api`, `API_COORDINATION_COMPLETED` from `events.api`, `API_COORDINATION_FAILED` from `events.api`, `API_COORDINATION_AGENT_RESOLVE_FAILED` from `events.api`, `CODE_RUNNER_EXECUTE_START` from `events.code_runner`, `DOCKER_EXECUTE_START` from `events.docker`, `MCP_INVOKE_START` from `events.mcp`, `SECURITY_EVALUATE_START` from `events.security`, `HR_HIRING_REQUEST_CREATED` from `events.hr`, `PERF_METRIC_RECORDED` from `events.performance`, `TRUST_EVALUATE_START` from `events.trust`, `PROMOTION_EVALUATE_START` from `events.promotion`, `PROMPT_BUILD_START` from `events.prompt`, `MEMORY_RETRIEVAL_START` from `events.memory`, `MEMORY_BACKEND_CONNECTED` from `events.memory`, `MEMORY_ENTRY_STORED` from `events.memory`, `MEMORY_BACKEND_SYSTEM_ERROR` from `events.memory`, `AUTONOMY_ACTION_AUTO_APPROVED` from `events.autonomy`, `TIMEOUT_POLICY_EVALUATED` from `events.timeout`, `PERSISTENCE_AUDIT_ENTRY_SAVED` from `events.persistence`, `TASK_ENGINE_STARTED` from `events.task_engine`, `COORDINATION_STARTED` from `events.coordination`, `COORDINATION_FACTORY_BUILT` from `events.coordination`, `COMMUNICATION_DISPATCH_START` from `events.communication`, `COMPANY_STARTED` from `events.company`, `CONFIG_LOADED` from `events.config`, `CORRELATION_ID_CREATED` from `events.correlation`, `DECOMPOSITION_STARTED` from `events.decomposition`, `DELEGATION_STARTED` from `events.delegation`, `EXECUTION_LOOP_START` from `events.execution`, `CHECKPOINT_SAVED` from `events.checkpoint`, `PERSISTENCE_CHECKPOINT_SAVED` from `events.persistence`, `GIT_OPERATION_START` from `events.git`, `PARALLEL_GROUP_START` from `events.parallel`, `PERSONALITY_LOADED` from `events.personality`, `QUOTA_CHECKED` from `events.quota`, `ROLE_ASSIGNED` from `events.role`, `ROUTING_STARTED` from `events.routing`, `SANDBOX_EXECUTE_START` from `events.sandbox`, `TASK_CREATED` from `events.task`, `TASK_ASSIGNMENT_STARTED` from `events.task_assignment`, `TASK_ROUTING_STARTED` from `events.task_routing`, `TEMPLATE_LOADED` from `events.template`, `TOOL_INVOKE_START` from `events.tool`, `TOOL_OUTPUT_WITHHELD` from `events.tool`, `WORKSPACE_CREATED` from `events.workspace`, `APPROVAL_GATE_ESCALATION_DETECTED` from `events.approval_gate`, `APPROVAL_GATE_ESCALATION_FAILED` from `events.approval_gate`, `APPROVAL_GATE_INITIALIZED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFIED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFY_FAILED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARKED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARK_FAILED` from `events.approval_gate`, `APPROVAL_GATE_PARK_TASKLESS` from `events.approval_gate`, `APPROVAL_GATE_RESUME_STARTED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_RESUMED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_DELETE_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_TRIGGERED` from `events.approval_gate`, `APPROVAL_GATE_NO_PARKED_CONTEXT` from `events.approval_gate`, `APPROVAL_GATE_LOOP_WIRING_WARNING` from `events.approval_gate`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider condensing the event names list.

The exhaustive inline list of 60+ event constants is comprehensive but impacts readability. Consider either:

  • Referencing the synthorg.observability.events module structure directly
  • Moving the full list to a separate reference document

This is a minor readability concern; the current form is functionally correct.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 190, The long inline list of event constants (e.g.,
PROVIDER_CALL_START, BUDGET_RECORD_ADDED, CFO_ANOMALY_DETECTED, MEETING_STARTED,
CLASSIFICATION_START, MEMORY_RETRIEVAL_START, TASK_CREATED,
APPROVAL_GATE_ESCALATION_DETECTED, etc.) hurts readability; condense this by
replacing the exhaustive list with a short explanatory sentence pointing to
synthorg.observability.events and an example import pattern (e.g., "from
synthorg.observability.events.<domain> import EVENT_CONSTANT"), or move the full
list into a separate reference document and link to it from the guideline;
update the CLAUDE.md section that currently enumerates the constants so it
contains only the module reference and an example, and add the full enumeration
to a referenced doc for maintainability.

…, and Codecov

Source fixes:
- ErrorDetail: message uses NotBlankStr, retry_after/retryable cross-field
  validator, error_code docstring clarified as "by convention 4-digit"
- ApiResponse/PaginatedResponse: model_validator rejects error_detail on
  success responses
- ApiError docstring: distinguish ClassVar vs instance attributes
- MappingProxyType wrapping on _CODE_CATEGORY_PREFIX and _STATUS_TO_ERROR_META
- _get_instance_id: log at DEBUG on fallback instead of silent pass
- _build_error_response: docstring documents instance auto-population
- Dead code: remove unreachable `or type(exc).default_message` branch
- Security: allow-list exc.headers (retry-after, www-authenticate, allow)
- Security: 401/403 handlers use fixed strings instead of echoing exc.detail
- Middleware: status_code=0 (int) instead of "unknown" (str) in structured
  logs; log WARNING on missing ASGI status key
- _CODE_CATEGORY_PREFIX comment: remove test code reference

Frontend type sync:
- Add ErrorDetail, ErrorCategory, ErrorCode types to types.ts
- Add error_detail field to ApiResponse/PaginatedResponse discriminated unions
- Add ApiRequestError class to client.ts carrying ErrorDetail
- Add getErrorDetail() helper to errors.ts

Test improvements:
- Module-level pytestmark in test_dto, test_exception_handlers,
  test_rate_limiter, test_middleware (consistency)
- Hoist in-function imports to module level in test_exception_handlers
  and test_rate_limiter
- New tests: retry_after negative rejection, retry_after/retryable
  consistency, error_detail on success rejected, _get_instance_id
  exception fallback, _build_error_response with retry_after,
  middleware correlation ID binding/clearing, _log_request_completion
  with status_code=None
- Fix asyncio.sleep(0.05) in test_concurrent_limit to asyncio.sleep(0)
- Fix auth handler test expectations for fixed strings

Documentation:
- CLAUDE.md: add API_REQUEST_COMPLETED to event constants list
Update client.property.test.ts to include error_detail field
in mock ApiResponse/PaginatedResponse objects, matching the
updated discriminated union types.
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 18:56 — with GitHub Actions Inactive
@Aureliolo Aureliolo merged commit 6612a99 into main Mar 15, 2026
32 checks passed
@Aureliolo Aureliolo deleted the feat/rfc-9457-errors branch March 15, 2026 19:01
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 19:01 — with GitHub Actions Inactive
Aureliolo added a commit that referenced this pull request Mar 15, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.2.6](v0.2.5...v0.2.6)
(2026-03-15)


### Features

* add intra-loop stagnation detector
([#415](#415))
([#458](#458))
([8e9f34f](8e9f34f))
* add RFC 9457 structured error responses (Phase 1)
([#457](#457))
([6612a99](6612a99)),
closes [#419](#419)
* implement AgentStateRepository for runtime state persistence
([#459](#459))
([5009da7](5009da7))
* **site:** add SEO essentials, contact form, early-access banner
([#467](#467))
([11b645e](11b645e)),
closes [#466](#466)


### Bug Fixes

* CLI improvements — config show, completion install, enhanced doctor,
Sigstore verification
([#465](#465))
([9e08cec](9e08cec))
* **site:** add reCAPTCHA v3, main landmark, and docs sitemap
([#469](#469))
([fa6d35c](fa6d35c))
* use force-tag-creation instead of manual tag creation hack
([#462](#462))
([2338004](2338004))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: implement RFC 9457 structured error responses for API

1 participant