Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent 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). (6)
🧰 Additional context used📓 Path-based instructions (2)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2026-03-15T11:48:14.867ZApplied to files:
📚 Learning: 2026-03-15T11:48:14.867ZApplied to files:
📚 Learning: 2026-03-19T07:09:59.660ZApplied to files:
📚 Learning: 2026-03-19T07:09:59.660ZApplied to files:
📚 Learning: 2026-03-19T07:09:59.660ZApplied to files:
🧬 Code graph analysis (2)tests/unit/api/test_server.py (1)
tests/unit/api/test_app.py (3)
🔇 Additional comments (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughCalls logging bootstrap during app startup, wires SYNTHORG_LOG_DIR/SYNTHORG_LOG_LEVEL handling, adds an access.log sink and routing, disables Uvicorn access logging/log_config, replaces per-timeout bus debug spam with periodic idle summaries, and updates docs and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Env as Environment (SYNTHORG_*)
participant App as create_app()
participant Bootstrap as _bootstrap_app_logging()
participant Logger as bootstrap_logging()
participant Sinks as File/Console Sinks
participant Server as uvicorn.run()
Env->>App: provide SYNTHORG_LOG_DIR / SYNTHORG_LOG_LEVEL
App->>Bootstrap: call with RootConfig
Bootstrap->>Bootstrap: validate/patch logging (log_dir, sinks)
Bootstrap->>Logger: bootstrap_logging(resolved RootConfig)
Logger->>Sinks: create/attach handlers (console + file sinks, routing)
Sinks-->>Logger: handlers attached / raise
Logger-->>Bootstrap: return success or raise
Bootstrap-->>App: patched RootConfig or error
App->>Server: start server with access_log=False, log_config=None
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Summary of ChangesHello, 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 enhances the application's logging infrastructure by fully activating a previously implemented structured logging pipeline. It ensures that all application logs are routed through a robust 8-sink system, improves Uvicorn integration by disabling its default logging in favor of structured logs, and reduces log verbosity by summarizing idle channel activity. Critical logging components are now enforced as startup requirements, and new environment variables provide flexible configuration options for log directories and console levels, making the system more observable and maintainable. Highlights
Using Gemini Code AssistThe 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
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 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. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request significantly enhances the logging pipeline by implementing a comprehensive 8-sink structured logging system. Key improvements include wiring the logging bootstrap into the application startup, introducing environment variables for log directory and console log level overrides, disabling Uvicorn's default access logger in favor of a richer structured one, and replacing verbose per-timeout debug logs with a periodic summary. Critical sink failures (audit.log, access.log) now result in hard startup errors, ensuring essential security and access records are not silently dropped. The changes are well-documented in docs/design/operations.md and thoroughly tested, demonstrating a strong focus on observability, robustness, and configurability.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/operations.md`:
- Around line 1189-1206: The Per-Logger Levels table in the docs is missing the
`synthorg.cli` entry; update the "Per-Logger Levels" table to include
`synthorg.cli` with default level `INFO` so it matches the
`_DEFAULT_LOGGER_LEVELS` constant in `src/synthorg/observability/setup.py`;
ensure the new row follows the same format as the others (Logger | Default
Level) and place it with the other `synthorg.*` entries.
- Around line 1236-1241: Replace the ASCII double hyphens used as dashes with an
em dash for typographic consistency in the observability settings description:
change "`--` changes the root logger level" to use an em dash after
`root_log_level` and similarly replace "`-- toggles correlation ID injection`"
after `enable_correlation`, and also ensure the console sink note uses an em
dash before `SYNTHORG_LOG_LEVEL`; locate these phrases near the
`SettingsService`, `root_log_level`, `enable_correlation`, and
`SYNTHORG_LOG_LEVEL` mentions and swap `--` for `—`.
In `@src/synthorg/api/app.py`:
- Line 405: Update the inline comment that currently reads "7+1 sinks" to say "8
sinks" (or "8 default sinks") to match the documentation and avoid confusion;
locate the comment near the logging activation line in app.py (the line starting
with "Activate the structured logging pipeline") and replace the phrase "7+1
sinks" with "8 sinks" while preserving the rest of the comment.
In `@src/synthorg/communication/bus_memory.py`:
- Around line 109-110: The idle tracking fields initialized in __init__
(_idle_poll_count and _last_idle_summary) are not reset when the bus is
restarted, which can cause an immediate idle summary on first poll; update the
start() method in the same class (e.g., BusMemory) to reset
self._idle_poll_count to 0 and set self._last_idle_summary = time.monotonic()
immediately after setting self._running = True so the idle state is
reinitialized on every start.
In `@tests/unit/communication/test_bus_memory.py`:
- Around line 738-751: The test TestIdleSummary uses real short timeouts
(bus.receive with timeout=0.01) which can be flaky; update the test to mock time
so the idle polling behavior is deterministic: patch the clock used by
InMemoryMessageBus (e.g., time.monotonic or the event loop time function used
inside InMemoryMessageBus) or inject a fake clock to advance time between
receive calls, then call bus.receive repeatedly and assert bus._idle_poll_count
== 5; keep references to InMemoryMessageBus, bus.receive, and
bus._idle_poll_count so you locate the code path that checks timeouts and
replace real timing with a controllable mock/clock.
- Around line 753-767: The test test_summary_emits_after_time_interval should
avoid directly mutating bus._last_idle_summary and instead mock time.monotonic()
for determinism; update the test in tests/unit/communication/test_bus_memory.py
to use the provided monkeypatch fixture to stub time.monotonic() (or a similar
time provider) so that when InMemoryMessageBus (instantiate via
InMemoryMessageBus and start) calls time.monotonic() it returns a value older
than the summary threshold, then call bus.receive("#general","agent-a",
timeout=0.01) and assert bus._idle_poll_count == 0; reference
InMemoryMessageBus, bus._last_idle_summary, and
test_summary_emits_after_time_interval when making the change.
🪄 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: 7b0fcdc4-a54c-4d09-ac91-11dbae1f2762
📒 Files selected for processing (15)
docker/.env.exampledocker/backend/Dockerfiledocker/compose.ymldocs/design/operations.mdsrc/synthorg/api/app.pysrc/synthorg/api/server.pysrc/synthorg/communication/bus_memory.pysrc/synthorg/observability/config.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/sinks.pytests/unit/api/test_app.pytests/unit/communication/test_bus_memory.pytests/unit/observability/test_config.pytests/unit/observability/test_setup.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 Sandbox
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (11)
docker/**/{Dockerfile*,*.yml,*.env*,.dockerignore}
📄 CodeRabbit inference engine (CLAUDE.md)
Docker config: all files in
docker/— Dockerfiles, compose,.env.example; single root.dockerignore; build context:.
Files:
docker/compose.ymldocker/backend/Dockerfile
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) — ruff enforces this on Python 3.14
Type hints required on all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones; for non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves — never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); use@computed_fieldfor derived values instead of storing + validating redundant fields; useNotBlankStrfor all identifier/name fields including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Line length: 88 characters (ruff)
Functions should be < 50 lines, files < 800 lines
Validate at system boundaries (user input, external APIs, config files)
Files:
src/synthorg/api/server.pytests/unit/observability/test_config.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pytests/unit/communication/test_bus_memory.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/config.pytests/unit/observability/test_setup.pytests/unit/api/test_app.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
**/{*.py,*.go}
📄 CodeRabbit inference engine (CLAUDE.md)
Handle errors explicitly, never silently swallow them
Files:
src/synthorg/api/server.pytests/unit/observability/test_config.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pytests/unit/communication/test_bus_memory.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/config.pytests/unit/observability/test_setup.pytests/unit/api/test_app.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code
Variable name for logger: alwayslogger(not_logger, notlog)
Event names: always use constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool). Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging: alwayslogger.info(EVENT, key=value)— neverlogger.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
DEBUG level logging for object creation, internal flow, entry/exit of key functions
Library reference: auto-generated from docstrings via mkdocstrings + Griffe (AST-based, no imports) indocs/api/
Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP, tool factory, sandbox factory)
Files:
src/synthorg/api/server.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/config.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
{src/synthorg/**/*.py,tests/**/*.py,**/*.md,web/**/{*.ts,*.js,*.vue}}
📄 CodeRabbit inference engine (CLAUDE.md)
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,large/medium/smallas aliases. Vendor names only in: (1) Operations design page, (2).claude/files, (3) third-party import paths/modules
Files:
src/synthorg/api/server.pytests/unit/observability/test_config.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pytests/unit/communication/test_bus_memory.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/config.pytests/unit/observability/test_setup.pydocs/design/operations.mdtests/unit/api/test_app.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Files:
src/synthorg/api/server.pysrc/synthorg/api/app.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async testing:asyncio_mode = "auto"— no manual@pytest.mark.asyncioneeded
Prefer@pytest.mark.parametrizefor testing similar cases
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
Files:
tests/unit/observability/test_config.pytests/unit/communication/test_bus_memory.pytests/unit/observability/test_setup.pytests/unit/api/test_app.py
src/synthorg/observability/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Files:
src/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/config.py
docs/design/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/design/*.md: When approved deviations occur, update the relevantdocs/design/page to reflect the new reality
Design spec pages: 7 pages indocs/design/— index, agents, organization, communication, engine, memory, operations
Files:
docs/design/operations.md
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Markdown: use for all documentation files (
docs/,site/, README, etc.)
Files:
docs/design/operations.md
src/synthorg/communication/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Communication package (communication/): message bus, dispatcher, messenger, channels, delegation, loop prevention, conflict resolution; meeting/ subpackage for meeting protocol (round-robin, position papers, structured phases), scheduler (frequency, participant resolver), orchestrator
Files:
src/synthorg/communication/bus_memory.py
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
📚 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 docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docker/compose.ymldocker/backend/Dockerfile
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to docker/Dockerfile.sandbox : Docker sandbox: `synthorg-sandbox` — Python 3.14 + Node.js + git, non-root (UID 10001), agent code execution sandbox
Applied to files:
docker/compose.ymldocker/backend/Dockerfiledocker/.env.example
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : Variable name for logger: always `logger` (not `_logger`, not `log`)
Applied to files:
docker/compose.ymlsrc/synthorg/observability/sinks.pysrc/synthorg/observability/setup.pysrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to docker/Dockerfile : Docker: 3-stage build (builder → setup → distroless runtime) for backend, Chainguard Python, non-root (UID 65532), CIS-hardened
Applied to files:
docker/backend/Dockerfile
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/sinks.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/config.pytests/unit/observability/test_setup.pydocs/design/operations.mdsrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
src/synthorg/observability/sinks.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
docker/.env.example
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
docker/.env.exampledocs/design/operations.mdtests/unit/api/test_app.pysrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/communication/**/*.py : Communication package (communication/): message bus, dispatcher, messenger, channels, delegation, loop prevention, conflict resolution; meeting/ subpackage for meeting protocol (round-robin, position papers, structured phases), scheduler (frequency, participant resolver), orchestrator
Applied to files:
src/synthorg/observability/events/communication.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : DEBUG level logging for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/observability/setup.pysrc/synthorg/api/app.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 : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/observability/setup.pysrc/synthorg/api/app.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:
src/synthorg/observability/setup.pytests/unit/observability/test_setup.pysrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/observability/setup.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 state transitions must log at INFO.
Applied to files:
src/synthorg/observability/setup.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:
src/synthorg/observability/setup.pytests/unit/api/test_app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/synthorg/observability/setup.pytests/unit/observability/test_setup.pysrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging` / `logging.getLogger()` / `print()` in application code
Applied to files:
src/synthorg/observability/setup.pytests/unit/api/test_app.pysrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings package (settings/): runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge (JSON serialization for Pydantic/collections), ConfigResolver (typed accessors), validation, registry, change notifications via message bus, SettingsSubscriber protocol, SettingsChangeDispatcher (polls `#settings` channel, routes to subscribers, restart_required filtering)
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/backup/**/*.py : Backup package (backup/): scheduled/manual/lifecycle backups of persistence DB, agent memory, company config. BackupService orchestrator, BackupScheduler (periodic asyncio task), RetentionManager (count + age pruning), tar.gz compression, SHA-256 checksums, manifest tracking, validated restore with atomic rollback and safety backup. handlers/ subpackage: ComponentHandler protocol + concrete handlers (PersistenceComponentHandler, MemoryComponentHandler, ConfigComponentHandler)
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
tests/unit/api/test_app.pysrc/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
tests/unit/api/test_app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/**/*.py : Use structured logging: always `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)`
Applied to files:
src/synthorg/api/app.py
📚 Learning: 2026-03-18T21:35:45.198Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:35:45.198Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/api/app.py
🧬 Code graph analysis (3)
tests/unit/communication/test_bus_memory.py (1)
src/synthorg/communication/bus_memory.py (5)
InMemoryMessageBus(88-666)start(117-138)subscribe(332-380)receive(425-489)publish(174-215)
tests/unit/observability/test_setup.py (2)
src/synthorg/observability/setup.py (2)
_apply_console_level_override(176-218)configure_logging(221-262)src/synthorg/observability/config.py (2)
LogConfig(116-190)SinkConfig(51-113)
src/synthorg/api/app.py (2)
src/synthorg/config/loader.py (1)
bootstrap_logging(477-492)src/synthorg/observability/config.py (1)
LogConfig(116-190)
🪛 LanguageTool
docs/design/operations.md
[typographical] ~1239-~1239: Consider using an em dash (—) instead of ‘--’.
Context: ... level - enable_correlation (boolean) -- toggles correlation ID injection Conso...
(TWO_HYPHENS)
🔇 Additional comments (27)
src/synthorg/observability/events/communication.py (1)
50-53: LGTM!The new
COMM_CHANNELS_IDLE_SUMMARYconstant follows the established naming convention and event taxonomy. It's appropriately placed in the "Receive" section alongside related events.src/synthorg/communication/bus_memory.py (4)
9-12: LGTM!The
timemodule import is needed fortime.monotonic(), andFinalis appropriately added for the module-level constant annotation.
57-59: LGTM!The 60-second interval is a reasonable choice for reducing idle-poll log spam while preserving periodic visibility. The
Finalannotation and private naming convention are appropriate.
488-489: LGTM!The simplified call to
_log_receive_nullwithout the timeout parameter aligns with the new periodic summary approach.
518-529: LGTM!The idle summary logic correctly:
- Increments the poll counter on each idle timeout
- Checks the time interval before emitting a summary
- Resets both counter and timestamp after logging
The modifications to
_idle_poll_countand_last_idle_summaryoutside the lock are safe in a single-threaded asyncio context since there are noawaitpoints between the read and write operations.tests/unit/communication/test_bus_memory.py (1)
769-779: LGTM!Good control test ensuring the idle summary mechanism doesn't interfere with normal message delivery flow.
docker/compose.yml (1)
22-22: LGTM!The
SYNTHORG_LOG_DIRenvironment variable is correctly configured with a sensible default inside the persisted volume. This aligns with the PR objective to make the log directory configurable via environment variables.src/synthorg/api/server.py (1)
58-59: LGTM!Disabling Uvicorn's access log (
access_log=False) and default logging config (log_config=None) correctly delegates HTTP access logging toRequestLoggingMiddleware, eliminating duplicate unstructured access lines per PR objective#560.tests/unit/observability/test_config.py (1)
276-295: LGTM!Test assertions correctly updated to reflect the new 8-sink configuration (console + 7 file sinks including the new
access.log).docker/backend/Dockerfile (1)
125-128: LGTM!The
--no-access-logflag in the Dockerfile CMD correctly mirrors theaccess_log=Falsesetting inserver.py, ensuring consistent behavior whether the container is started directly or via the Python entry point. The comment clearly explains the rationale.src/synthorg/observability/sinks.py (1)
33-33: LGTM!Adding
access.logto the routing map with prefixsynthorg.api.correctly ensures HTTP access logs fromRequestLoggingMiddlewareand other API modules are captured in the dedicated access log sink.docker/.env.example (1)
29-33: LGTM!Documentation for
SYNTHORG_LOG_DIRandSYNTHORG_LOG_LEVELis clear and consistent with the implementation. The commented-outSYNTHORG_LOG_LEVELcorrectly indicates it's an optional override with valid values listed.src/synthorg/observability/config.py (2)
8-9: LGTM!Docstring correctly updated to reflect the eight-sink layout.
241-247: LGTM!The
access.logsink is correctly configured withINFOlevel, rotation, and JSON formatting—consistent with other file sinks inDEFAULT_SINKS.src/synthorg/observability/setup.py (3)
120-157: LGTM!The critical sink failure handling is well-designed:
audit.logandaccess.logcorrectly identified as critical sinks that must not fail silently- Warning is flushed to stderr before raising
RuntimeError, ensuring the diagnostic message is visible- Non-critical sinks continue to degrade gracefully
The PEP 758 except syntax (
except OSError, RuntimeError, ValueError:) is correct per coding guidelines.
176-218: LGTM!The
_apply_console_level_overridefunction is well-implemented:
- Correctly parses and validates
SYNTHORG_LOG_LEVELwith helpful error messages listing valid values- Uses
model_copy(update=...)per guidelines for updating frozen Pydantic models- Appropriately warns when the env var is set but no console sink exists
- Falls back to
INFOon invalid values rather than failing startup
225-238: LGTM!The updated docstring clearly documents the
SYNTHORG_LOG_LEVELbehavior, and the override is correctly applied before any logging configuration to ensure the env var takes effect.tests/unit/api/test_app.py (3)
9-16: LGTM!The imports are correctly added to support testing the
_bootstrap_app_loggingfunction and constructing test configs withLogConfigandDEFAULT_SINKS.
420-425: LGTM!The monkeypatch correctly isolates
structlog.testing.capture_logs()from the bootstrap logging side effects, and the comment clearly documents the reasoning.
916-973: LGTM!Comprehensive test coverage for all three branches of
_bootstrap_app_logging:
- No env var → original config passed through
- Env var with existing
logging→log_diroverride applied- Env var without existing
logging→ newLogConfigcreated withDEFAULT_SINKSThe test assertions correctly verify both the call count and the modified config properties.
tests/unit/observability/test_setup.py (3)
16-20: LGTM!The import of
_apply_console_level_overrideenables testing the internal helper function, which is appropriate for unit tests that need to verify specific branching logic.
58-58: LGTM!The handler count of 8 correctly reflects the updated sink layout: console + 7 file sinks (synthorg.log, audit.log, errors.log, agent_activity.log, cost_usage.log, debug.log, access.log).
Also applies to: 111-111
200-277: LGTM!Thorough test coverage for
_apply_console_level_override:
- Identity return when env var unset
- Valid level override
- Invalid level fallback with stderr warning
- File sinks remain unaffected
- Missing console sink warning
The use of
capsysto verify stderr warnings is appropriate.docs/design/operations.md (1)
1135-1157: LGTM!The sink layout table accurately documents all 8 sinks with their types, levels, formats, routing, and descriptions. The routing patterns match the implementation in
_LoggerNameFilter.src/synthorg/api/app.py (3)
11-11: LGTM!The new imports are correctly added:
sysfor stderr access in the error pathbootstrap_loggingfrom the config moduleDEFAULT_SINKSandLogConfigfor constructing fallback logging configAlso applies to: 52-52, 60-60
334-358: LGTM!The function correctly handles all three cases:
SYNTHORG_LOG_DIRset with existingloggingconfig → patcheslog_dirSYNTHORG_LOG_DIRset withoutloggingconfig → creates minimalRootConfigwithDEFAULT_SINKS- No env override → passes through original config
The use of
model_copy(update=...)for immutable updates aligns with the frozen Pydantic model pattern.
404-419: LGTM!The error handling correctly uses
print()to stderr (appropriate since the logging system itself is failing) and provides actionable guidance mentioningSYNTHORG_LOG_DIR,SYNTHORG_LOG_LEVEL, and the config file'sloggingsection. Re-raising preserves the stack trace for debugging.
| Two observability settings are runtime-editable via `SettingsService`: | ||
|
|
||
| - `root_log_level` (enum: debug/info/warning/error/critical) -- changes the root logger level | ||
| - `enable_correlation` (boolean) -- toggles correlation ID injection | ||
|
|
||
| Console sink level can also be overridden via `SYNTHORG_LOG_LEVEL` env var. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Use em dash for consistency.
The documentation uses em dashes (—) consistently elsewhere in the file (e.g., line 1138). Consider replacing -- with — for typographical consistency.
📝 Suggested fix
- `root_log_level` (enum: debug/info/warning/error/critical) -- changes the root logger level
-- `enable_correlation` (boolean) -- toggles correlation ID injection
+- `root_log_level` (enum: debug/info/warning/error/critical) — changes the root logger level
+- `enable_correlation` (boolean) — toggles correlation ID injection📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Two observability settings are runtime-editable via `SettingsService`: | |
| - `root_log_level` (enum: debug/info/warning/error/critical) -- changes the root logger level | |
| - `enable_correlation` (boolean) -- toggles correlation ID injection | |
| Console sink level can also be overridden via `SYNTHORG_LOG_LEVEL` env var. | |
| Two observability settings are runtime-editable via `SettingsService`: | |
| - `root_log_level` (enum: debug/info/warning/error/critical) — changes the root logger level | |
| - `enable_correlation` (boolean) — toggles correlation ID injection | |
| Console sink level can also be overridden via `SYNTHORG_LOG_LEVEL` env var. |
🧰 Tools
🪛 LanguageTool
[typographical] ~1239-~1239: Consider using an em dash (—) instead of ‘--’.
Context: ... level - enable_correlation (boolean) -- toggles correlation ID injection Conso...
(TWO_HYPHENS)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/operations.md` around lines 1236 - 1241, Replace the ASCII double
hyphens used as dashes with an em dash for typographic consistency in the
observability settings description: change "`--` changes the root logger level"
to use an em dash after `root_log_level` and similarly replace "`-- toggles
correlation ID injection`" after `enable_correlation`, and also ensure the
console sink note uses an em dash before `SYNTHORG_LOG_LEVEL`; locate these
phrases near the `SettingsService`, `root_log_level`, `enable_correlation`, and
`SYNTHORG_LOG_LEVEL` mentions and swap `--` for `—`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #572 +/- ##
==========================================
+ Coverage 92.57% 92.64% +0.07%
==========================================
Files 544 544
Lines 26872 26931 +59
Branches 2574 2582 +8
==========================================
+ Hits 24876 24951 +75
+ Misses 1584 1568 -16
Partials 412 412 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4382081 to
e78a6c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/app.py`:
- Around line 334-370: The helper _bootstrap_app_logging currently mutates
nothing visible to callers: change its signature to return the effective
RootConfig and update logic so it returns effective_config when no
SYNTHORG_LOG_DIR is set (after calling bootstrap_logging), and returns the
patched RootConfig (the variable patched) after calling bootstrap_logging when a
log_dir override is applied; then update callers (e.g., create_app and any code
that assigns AppState.config) to use the returned config so
AppState.config.logging reflects the override. Apply the same
return-the-patched-config change to the other identical logging-patching block
in this file that handles SYNTHORG_LOG_DIR.
In `@src/synthorg/observability/setup.py`:
- Around line 145-158: The exception handler in setup.py currently always prints
"This sink will be skipped" even for critical sinks, which is misleading when
you then raise RuntimeError; update the logic in the except block that handles
(OSError, RuntimeError, ValueError) so that you first check if sink.file_path is
in _critical_sinks and, for critical sinks, print a clear refusal-to-start
message (e.g., "Critical log sink '{sink.file_path}' could not be initialised.
Refusing to start.") and then raise the RuntimeError from exc, while for
non-critical sinks keep the existing "will be skipped" wording; refer to the
variables and symbols sink, sink.file_path, _critical_sinks and the existing
RuntimeError raise to locate where to change the printed message and control
flow.
In `@tests/unit/api/test_server.py`:
- Around line 20-29: The test currently invokes the real app factory path; stub
the app factory by patching synthorg.api.server.create_app to a lightweight stub
(e.g., a lambda that returns a simple MagicMock or minimal ASGI app) before
importing or calling run_server so the unit test remains isolated; specifically
replace create_app in the test with a stub that accepts a RootConfig and returns
a dummy app, keep the existing monkeypatch for _bootstrap_app_logging and the
patched uvicorn.run assertion intact.
🪄 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: bda31ca1-d0ea-44af-af3c-9c32f4c79296
📒 Files selected for processing (22)
CLAUDE.mdcli/internal/compose/compose.yml.tmplcli/testdata/compose_custom_ports.ymlcli/testdata/compose_default.ymlcli/testdata/compose_digest_pins.ymldocker/.env.exampledocker/backend/Dockerfiledocker/compose.ymldocs/design/operations.mdsrc/synthorg/api/app.pysrc/synthorg/api/server.pysrc/synthorg/communication/bus_memory.pysrc/synthorg/observability/config.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/sinks.pytests/unit/api/test_app.pytests/unit/api/test_server.pytests/unit/communication/test_bus_memory.pytests/unit/observability/test_config.pytests/unit/observability/test_setup.pytests/unit/observability/test_sink_routing.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). (5)
- GitHub Check: Test (Python 3.14)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
docker/**
📄 CodeRabbit inference engine (CLAUDE.md)
All Docker files in docker/ — Dockerfiles, compose, .env.example. Use single root .dockerignore. All images build with context: . (from repo root).
Files:
docker/compose.ymldocker/backend/Dockerfile
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations.
Useexcept A, B:syntax (no parentheses) for exception handling — PEP 758 except syntax, enforced by ruff on Python 3.14.
All public functions require type hints — mypy strict mode enforced.
Docstrings must use Google style and are required on all public classes and functions — enforced by ruff D rules.
Files:
src/synthorg/api/server.pytests/unit/observability/test_sink_routing.pytests/unit/observability/test_config.pysrc/synthorg/observability/events/communication.pytests/unit/api/test_server.pysrc/synthorg/observability/sinks.pytests/unit/communication/test_bus_memory.pysrc/synthorg/observability/config.pysrc/synthorg/observability/setup.pysrc/synthorg/communication/bus_memory.pysrc/synthorg/api/app.pytests/unit/observability/test_setup.pytests/unit/api/test_app.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement.
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields. Use NotBlankStr (from core.types) for all identifier/name fields.
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Line length: 88 characters (ruff).
Functions must be less than 50 lines; files must be less than 800 lines.
Handle errors explicitly, never silently swallow them.
Validate at system boundaries (user input, external APIs, config files).
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, large/medium/small as aliases. Tests must use test-provider, test-small-001, etc.
Files:
src/synthorg/api/server.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/config.pysrc/synthorg/observability/setup.pysrc/synthorg/communication/bus_memory.pysrc/synthorg/api/app.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__).
Never useimport logging/logging.getLogger()/print()in application code.
Always useloggeras the variable name (not_logger, notlog).
Event names must always use constants from the domain-specific module under synthorg.observability.events. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Use structured kwargs in logger calls: alwayslogger.info(EVENT, key=value)— neverlogger.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.
DEBUG logging for object creation, internal flow, entry/exit of key functions.
All provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code.
RetryConfig and RateLimiterConfig are set per-provider in ProviderConfig.
Retryable errors (is_retryable=True) include: RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError. Non-retryable errors raise immediately.
RetryExhaustedError signals that all retries failed — the engine layer catches this to trigger fallback chains.
Files:
src/synthorg/api/server.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/config.pysrc/synthorg/observability/setup.pysrc/synthorg/communication/bus_memory.pysrc/synthorg/api/app.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow.
Async tests use asyncio_mode = 'auto' — no manual@pytest.mark.asyncioneeded.
Test timeout: 30 seconds per test.
Prefer@pytest.mark.parametrizefor testing similar cases.
Use Hypothesis for property-based testing with@given+@settings. Hypothesis profiles: ci (50 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.
NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins.
Files:
tests/unit/observability/test_sink_routing.pytests/unit/observability/test_config.pytests/unit/api/test_server.pytests/unit/communication/test_bus_memory.pytests/unit/observability/test_setup.pytests/unit/api/test_app.py
🧠 Learnings (15)
📓 Common learnings
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 : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`.
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`).
📚 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 docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docker/compose.ymldocker/backend/Dockerfile
📚 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 : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
cli/testdata/compose_default.ymlcli/testdata/compose_custom_ports.ymlcli/testdata/compose_digest_pins.ymlCLAUDE.mdsrc/synthorg/observability/setup.pysrc/synthorg/api/app.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:
src/synthorg/observability/events/communication.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to docker/Dockerfile : Docker backend: 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened.
Applied to files:
docker/backend/Dockerfile
📚 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:
tests/unit/api/test_server.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Always use `logger` as the variable name (not `_logger`, not `log`).
Applied to files:
src/synthorg/observability/sinks.pysrc/synthorg/observability/setup.pydocs/design/operations.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/setup.pydocs/design/operations.mdsrc/synthorg/api/app.pytests/unit/observability/test_setup.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.mdsrc/synthorg/observability/setup.pydocs/design/operations.mdsrc/synthorg/api/app.pytests/unit/observability/test_setup.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging` / `logging.getLogger()` / `print()` in application code.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/setup.pysrc/synthorg/api/app.pytests/unit/observability/test_setup.pytests/unit/api/test_app.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Docs source in docs/ (Markdown, built with Zensical). Design spec: docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture: docs/architecture/. Roadmap: docs/roadmap/. Security: docs/security.md. Licensing: docs/licensing.md. Reference: docs/reference/. Custom templates: docs/overrides/.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins.
Applied to files:
tests/unit/communication/test_bus_memory.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level.
Applied to files:
src/synthorg/observability/setup.pydocs/design/operations.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/observability/setup.pysrc/synthorg/api/app.pytests/unit/api/test_app.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 state transitions must log at INFO.
Applied to files:
src/synthorg/observability/setup.pysrc/synthorg/api/app.py
🧬 Code graph analysis (6)
tests/unit/api/test_server.py (2)
src/synthorg/config/schema.py (1)
RootConfig(461-730)src/synthorg/api/server.py (1)
run_server(21-60)
tests/unit/communication/test_bus_memory.py (1)
src/synthorg/communication/bus_memory.py (5)
InMemoryMessageBus(88-668)start(117-140)subscribe(334-382)receive(427-491)stop(142-156)
src/synthorg/observability/setup.py (1)
src/synthorg/observability/config.py (2)
LogConfig(116-190)SinkConfig(51-113)
src/synthorg/api/app.py (2)
src/synthorg/config/loader.py (1)
bootstrap_logging(477-492)src/synthorg/observability/config.py (1)
LogConfig(116-190)
tests/unit/observability/test_setup.py (2)
src/synthorg/observability/setup.py (3)
_apply_console_level_override(177-221)_attach_handlers(113-158)configure_logging(224-271)src/synthorg/observability/config.py (2)
LogConfig(116-190)SinkConfig(51-113)
tests/unit/api/test_app.py (3)
src/synthorg/api/app.py (2)
_bootstrap_app_logging(334-370)create_app(373-576)src/synthorg/config/schema.py (1)
RootConfig(461-730)src/synthorg/observability/config.py (1)
LogConfig(116-190)
🪛 LanguageTool
docs/design/operations.md
[typographical] ~1241-~1241: Consider using an em dash (—) instead of ‘--’.
Context: ... level - enable_correlation (boolean) -- toggles correlation ID injection Conso...
(TWO_HYPHENS)
🔇 Additional comments (17)
cli/testdata/compose_default.yml (1)
18-18: LGTM!The
SYNTHORG_LOG_DIRenvironment variable is correctly added, pointing to/data/logswhich is within the mountedsynthorg-datavolume at/data. This aligns with the logging bootstrap changes.cli/internal/compose/compose.yml.tmpl (1)
18-18: LGTM!The hardcoded
/data/logspath is appropriate for CLI-generated compose files since the volume structure (synthorg-data:/data) is fixed. This contrasts withdocker/compose.ymlwhich uses${SYNTHORG_LOG_DIR:-/data/logs}for development flexibility.docker/compose.yml (1)
22-23: LGTM!Both logging environment variables correctly use shell parameter expansion with sensible defaults. The
env_file: .envdirective (line 16) enables easy override without modifying the compose file.tests/unit/observability/test_config.py (2)
277-277: LGTM!Sink count assertion correctly updated to 8, reflecting the addition of
access.logto the default sink configuration.
293-295: LGTM!The
test_valid_as_log_configassertion is properly updated to match the new 8-sink default configuration.src/synthorg/api/server.py (1)
58-59: LGTM — Correct Uvicorn integration for structured logging.Disabling
access_logprevents duplicate HTTP request logs (handled byRequestLoggingMiddleware), andlog_config=Noneprevents Uvicorn from overriding the bootstrap structlog configuration. This correctly implements issue#560.tests/unit/observability/test_sink_routing.py (1)
87-90: LGTM!The new test correctly validates that
access.logis in the routing table with thesynthorg.api.prefix, ensuring HTTP access events are routed to the dedicated access log sink.cli/testdata/compose_digest_pins.yml (1)
18-18: LGTM!
SYNTHORG_LOG_DIRadded consistently with other CLI testdata compose files.cli/testdata/compose_custom_ports.yml (1)
18-18: LGTM!
SYNTHORG_LOG_DIRadded consistently with other CLI testdata compose files.docker/backend/Dockerfile (1)
125-128: Good access-log suppression at runtime command.Disabling Uvicorn’s default access logger here is the right complement to middleware-driven structured access logging.
src/synthorg/observability/sinks.py (1)
33-33: Access sink routing is wired correctly.Adding
access.logto_SINK_ROUTINGcleanly reuses the existing prefix filter mechanism.src/synthorg/observability/events/communication.py (1)
53-53: Event constant change looks clean.
COMM_CHANNELS_IDLE_SUMMARYis a clear replacement for periodic idle telemetry.docker/.env.example (1)
29-34: Env documentation update is solid.Both log directory and console-level override are documented clearly and with valid value guidance.
src/synthorg/observability/config.py (2)
8-9: Eight-sink docstring note is accurate.The module docs now correctly describe the default sink layout.
241-247:access.logdefault sink addition is well-formed.Configuration mirrors other file sinks and cleanly extends
DEFAULT_SINKS.CLAUDE.md (1)
117-117: Package-structure documentation updates are clear and aligned.These additions capture the new bootstrap and 8-sink observability behavior well.
Also applies to: 131-131
tests/unit/communication/test_bus_memory.py (1)
738-817: Idle-summary test suite is deterministic and complete.Great use of mocked monotonic time and zero-timeout polling to validate behavior without flaky timing dependence.
…ress spam, integrate Uvicorn Wire bootstrap_logging() into create_app() so the comprehensive 8-sink structured logging system (console + synthorg.log + audit.log + errors.log + agent_activity.log + cost_usage.log + debug.log + access.log) actually activates at startup. Previously all logs fell through to Uvicorn's default stdout handler despite the full pipeline being implemented. - Wire bootstrap_logging() at top of create_app() with SYNTHORG_LOG_DIR env var support for Docker volume paths (#559) - Disable Uvicorn access log (RequestLoggingMiddleware already provides richer structured access logging) and add access.log sink (#560) - Replace per-timeout channel debug spam (41% of all log output) with periodic batch summary every 60 idle polls (#561) - Add SYNTHORG_LOG_LEVEL env var to override console sink level (#562) - Add Observability and Logging section to operations design spec (#563) Closes #559, closes #560, closes #561, closes #562, closes #563 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 10 agents, 14 findings addressed: - Remove dead COMM_RECEIVE_TIMEOUT constant (no longer used) - Replace count-based idle summary with time-based interval (robust regardless of concurrent subscriber count) - Change access.log sink level from DEBUG to INFO (match intent) - Fix module docstring: "seven-sink" -> "eight-sink" - Fix design spec event module count: 54 -> 50 - Warn on stderr when SYNTHORG_LOG_LEVEL set but no CONSOLE sink - Add flush=True to all stderr warning prints - Critical sink failure (audit.log, access.log) is now a hard error - Wrap _bootstrap_app_logging in try/except with diagnostic stderr - Remove redundant UVICORN_ACCESS_LOG env var from Dockerfile - Move deferred import to top-level in app.py - Add tests for _apply_console_level_override (5 cases) - Add tests for _bootstrap_app_logging (3 branches) - Add tests for idle summary behavior (3 cases) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mini Security: - Validate SYNTHORG_LOG_DIR for path traversal before model_copy (model_copy bypasses Pydantic validators) - Bind exception in _attach_handlers except clause for proper chaining Bugs: - Fix false "no CONSOLE sink" warning when SYNTHORG_LOG_LEVEL matches current level (use explicit flag instead of equality check) - Reset idle poll state on bus restart to prevent immediate summary - Chain original exception in critical sink failure (from exc, not None) - Add flush=True to _clear_root_handlers stderr print - Collapse _bootstrap_app_logging from 3 branches to 2 (removes bare RootConfig construction that discarded non-logging config) Docs: - Fix redacted token in operations.md ([REDACTED] -> **REDACTED**) - Add missing synthorg.cli to Per-Logger Levels table - Update CLAUDE.md api/ and observability/ package descriptions - Add Raises section to configure_logging docstring - Fix "7+1 sinks" comment to "8 sinks" Infra: - Add SYNTHORG_LOG_DIR to CLI compose template + golden files - Add SYNTHORG_LOG_LEVEL to docker/compose.yml environment block Tests (11 new): - Critical sink failure: 4 tests (non-critical skip, audit raises, access raises, cause chain preserved) - Bootstrap failure: 1 test (create_app prints CRITICAL and re-raises) - Path traversal: 1 test (SYNTHORG_LOG_DIR with '..' raises ValueError) - Whitespace LOG_DIR: 1 test (treated as unset) - access.log routing: 1 test in TestSinkRoutingTable - Server params: 1 test (uvicorn access_log=False, log_config=None) - Log level integration: 1 test (SYNTHORG_LOG_LEVEL end-to-end) - Idle summary: mock time.monotonic() for determinism, reference _IDLE_SUMMARY_INTERVAL_SECONDS constant, add restart reset test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mypy flagged _bm.time as not explicitly exported from the bus_memory module. Switch to importing the time module directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eading message, isolate test - _bootstrap_app_logging now returns the (possibly patched) RootConfig so create_app's effective_config reflects the SYNTHORG_LOG_DIR override - Restructure _attach_handlers except block: critical sinks print "Refusing to start" (not "will be skipped") before raising - Stub create_app in test_server.py to avoid running the full app factory - Update _bootstrap_app_logging tests to assert on return values Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e78a6c5 to
449df2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/backend/Dockerfile`:
- Around line 125-128: The Docker CMD currently starts Uvicorn with
"--no-access-log" but still allows Uvicorn's default LOGGING_CONFIG to be
applied, bypassing RequestLoggingMiddleware; fix by either adding the CLI flag
"--log-config /dev/null" to the existing CMD that runs "/app/.venv/bin/uvicorn
synthorg.api.app:create_app --factory --no-access-log" so the default logging
config is suppressed, or refactor startup so that synthorg.api.app calls
uvicorn.run(..., log_config=None) (and remove the Docker CMD invocation)
ensuring all logs flow through RequestLoggingMiddleware.
In `@docs/design/operations.md`:
- Around line 1146-1155: The current sink routing sends all synthorg.api.*
events to access.log which also captures non-request events like API_APP_STARTUP
and API_APP_SHUTDOWN; update the routing so access.log only receives actual HTTP
access entries by either using a dedicated logger name (e.g.,
synthorg.api.access or synthorg.api.http_access) in the code that emits request
logs, or add an event-level filter to exclude lifecycle events (API_APP_STARTUP,
API_APP_SHUTDOWN) from the access.log sink; change the logger used in
src/synthorg/api/app.py where startup/shutdown are emitted or adjust the sink
routes in the logging configuration to reference the new namespace/filter so
access.log remains request-only.
In `@src/synthorg/api/app.py`:
- Around line 429-435: The app currently applies logging settings only at
startup via _bootstrap_app_logging(effective_config) and
_build_settings_dispatcher() wires only provider/memory/backup subscribers so
runtime changes to root_log_level or enable_correlation never reconfigure
logging; update _build_settings_dispatcher (or add a dedicated settings
subscriber) to subscribe the SettingsService to changes for the logging keys
(e.g., "root_log_level", "enable_correlation") and, on change, invoke a
reconfiguration path that reapplies _bootstrap_app_logging or a smaller
reconfigure_logging(effective_config) helper using the new settings (ensuring it
runs safely from the dispatcher and updates existing logger sinks/correlation
behavior without requiring process restart).
In `@src/synthorg/observability/setup.py`:
- Line 145: Replace the tuple form of the except clause so it uses the tupleless
union syntax: change the current "except (OSError, RuntimeError, ValueError) as
exc:" to "except OSError | RuntimeError | ValueError as exc:" (keeping the same
exc variable and handling logic) in the try/except block in
src/synthorg/observability/setup.py so Ruff/Python 3.14–compatible syntax is
used.
In `@tests/unit/api/test_app.py`:
- Around line 917-1028: Add a create_app-level assertion to ensure the patched
logging config from _bootstrap_app_logging is preserved on the created app: set
SYNTHORG_LOG_DIR in the test, let _bootstrap_app_logging return the patched
RootConfig, call create_app(), then assert the created app's AppState stored
under app.state["app_state"] has config.logging.log_dir equal to the
SYNTHORG_LOG_DIR value; reference create_app, _bootstrap_app_logging and the
AppState access via app.state["app_state"].config.logging.log_dir to locate
where to add the assertion.
In `@tests/unit/api/test_server.py`:
- Around line 5-11: Add the standard 30s timeout marker for this module by
defining a module-level pytestmark variable in tests/unit/api/test_server.py so
the tests get a 30s timeout; e.g. add pytestmark = [pytest.mark.unit,
pytest.mark.timeout(30)] near the top (adjacent to the existing import and the
TestRunServerUvicornParams class) so the timeout applies without changing
individual test functions.
- Around line 31-33: Remove the redundant pre-assert that checks
call_kwargs.kwargs.get("access_log") is False or (len(call_kwargs.args) > 0) in
the call verification block of tests/unit/api/test_server.py; instead rely on
the existing assertions later that validate the exact kwargs/args contract (the
assertions at Lines 35 and 38), so delete the line referencing
call_kwargs.kwargs.get("access_log") and its surrounding tautological
conditional to avoid duplicate coverage while leaving the later explicit checks
for call_kwargs.args and call_kwargs.kwargs intact.
🪄 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: e03763c2-9d17-4c09-b582-78025625f582
📒 Files selected for processing (22)
CLAUDE.mdcli/internal/compose/compose.yml.tmplcli/testdata/compose_custom_ports.ymlcli/testdata/compose_default.ymlcli/testdata/compose_digest_pins.ymldocker/.env.exampledocker/backend/Dockerfiledocker/compose.ymldocs/design/operations.mdsrc/synthorg/api/app.pysrc/synthorg/api/server.pysrc/synthorg/communication/bus_memory.pysrc/synthorg/observability/config.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/observability/sinks.pytests/unit/api/test_app.pytests/unit/api/test_server.pytests/unit/communication/test_bus_memory.pytests/unit/observability/test_config.pytests/unit/observability/test_setup.pytests/unit/observability/test_sink_routing.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). (6)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
docker/**
📄 CodeRabbit inference engine (CLAUDE.md)
All Docker files in docker/ — Dockerfiles, compose, .env.example. Use single root .dockerignore. All images build with context: . (from repo root).
Files:
docker/compose.ymldocker/backend/Dockerfile
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations.
Useexcept A, B:syntax (no parentheses) for exception handling — PEP 758 except syntax, enforced by ruff on Python 3.14.
All public functions require type hints — mypy strict mode enforced.
Docstrings must use Google style and are required on all public classes and functions — enforced by ruff D rules.
Files:
src/synthorg/api/server.pytests/unit/observability/test_sink_routing.pytests/unit/observability/test_config.pysrc/synthorg/observability/config.pysrc/synthorg/observability/sinks.pytests/unit/api/test_server.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pytests/unit/communication/test_bus_memory.pysrc/synthorg/api/app.pytests/unit/api/test_app.pytests/unit/observability/test_setup.pysrc/synthorg/communication/bus_memory.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement.
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields. Use NotBlankStr (from core.types) for all identifier/name fields.
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Line length: 88 characters (ruff).
Functions must be less than 50 lines; files must be less than 800 lines.
Handle errors explicitly, never silently swallow them.
Validate at system boundaries (user input, external APIs, config files).
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, large/medium/small as aliases. Tests must use test-provider, test-small-001, etc.
Files:
src/synthorg/api/server.pysrc/synthorg/observability/config.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__).
Never useimport logging/logging.getLogger()/print()in application code.
Always useloggeras the variable name (not_logger, notlog).
Event names must always use constants from the domain-specific module under synthorg.observability.events. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Use structured kwargs in logger calls: alwayslogger.info(EVENT, key=value)— neverlogger.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.
DEBUG logging for object creation, internal flow, entry/exit of key functions.
All provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code.
RetryConfig and RateLimiterConfig are set per-provider in ProviderConfig.
Retryable errors (is_retryable=True) include: RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError. Non-retryable errors raise immediately.
RetryExhaustedError signals that all retries failed — the engine layer catches this to trigger fallback chains.
Files:
src/synthorg/api/server.pysrc/synthorg/observability/config.pysrc/synthorg/observability/sinks.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/setup.pysrc/synthorg/api/app.pysrc/synthorg/communication/bus_memory.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow.
Async tests use asyncio_mode = 'auto' — no manual@pytest.mark.asyncioneeded.
Test timeout: 30 seconds per test.
Prefer@pytest.mark.parametrizefor testing similar cases.
Use Hypothesis for property-based testing with@given+@settings. Hypothesis profiles: ci (50 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.
NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins.
Files:
tests/unit/observability/test_sink_routing.pytests/unit/observability/test_config.pytests/unit/api/test_server.pytests/unit/communication/test_bus_memory.pytests/unit/api/test_app.pytests/unit/observability/test_setup.py
🧠 Learnings (15)
📓 Common learnings
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 : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging` / `logging.getLogger()` / `print()` in application code.
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`).
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`.
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.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level.
📚 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 docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docker/compose.ymldocker/backend/Dockerfile
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to docker/Dockerfile : Docker backend: 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened.
Applied to files:
docker/backend/Dockerfile
📚 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 : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
cli/testdata/compose_digest_pins.ymlcli/testdata/compose_custom_ports.ymlcli/testdata/compose_default.ymlsrc/synthorg/observability/setup.pysrc/synthorg/api/app.pyCLAUDE.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Always use `logger` as the variable name (not `_logger`, not `log`).
Applied to files:
src/synthorg/observability/sinks.pydocs/design/operations.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 : 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:
src/synthorg/observability/events/communication.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Event names must always use constants from the domain-specific module under synthorg.observability.events. Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/communication.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/observability/setup.pytests/unit/api/test_app.py
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level.
Applied to files:
src/synthorg/observability/setup.pydocs/design/operations.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging` / `logging.getLogger()` / `print()` in application code.
Applied to files:
src/synthorg/observability/setup.pysrc/synthorg/api/app.pyCLAUDE.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 : 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:
src/synthorg/observability/setup.pydocs/design/operations.mdsrc/synthorg/api/app.pytests/unit/observability/test_setup.pyCLAUDE.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`.
Applied to files:
src/synthorg/observability/setup.pydocs/design/operations.mdsrc/synthorg/api/app.pytests/unit/observability/test_setup.pyCLAUDE.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins.
Applied to files:
tests/unit/communication/test_bus_memory.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 state transitions must log at INFO.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:09:59.660Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:09:59.660Z
Learning: Docs source in docs/ (Markdown, built with Zensical). Design spec: docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture: docs/architecture/. Roadmap: docs/roadmap/. Security: docs/security.md. Licensing: docs/licensing.md. Reference: docs/reference/. Custom templates: docs/overrides/.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (5)
tests/unit/api/test_server.py (2)
src/synthorg/config/schema.py (1)
RootConfig(461-730)src/synthorg/api/server.py (1)
run_server(21-60)
src/synthorg/observability/setup.py (1)
src/synthorg/observability/config.py (2)
LogConfig(116-190)SinkConfig(51-113)
tests/unit/communication/test_bus_memory.py (1)
src/synthorg/communication/bus_memory.py (6)
InMemoryMessageBus(88-668)start(117-140)subscribe(334-382)receive(427-491)publish(176-217)stop(142-156)
tests/unit/api/test_app.py (3)
src/synthorg/api/app.py (2)
_bootstrap_app_logging(334-382)create_app(385-588)src/synthorg/config/schema.py (1)
RootConfig(461-730)src/synthorg/observability/config.py (1)
LogConfig(116-190)
tests/unit/observability/test_setup.py (1)
src/synthorg/observability/setup.py (3)
_apply_console_level_override(184-228)_attach_handlers(113-165)configure_logging(231-278)
🪛 LanguageTool
docs/design/operations.md
[typographical] ~1241-~1241: Consider using an em dash (—) instead of ‘--’.
Context: ... level - enable_correlation (boolean) -- toggles correlation ID injection Conso...
(TWO_HYPHENS)
🔇 Additional comments (14)
docker/.env.example (1)
29-33: Good env surface for logging controls.
SYNTHORG_LOG_DIRand the documentedSYNTHORG_LOG_LEVELvalues are clear and align with the runtime behavior described in this PR.CLAUDE.md (1)
117-117: Docs update is aligned and specific.The updated
api/andobservability/bullets accurately capture the logging bootstrap + 8-sink routing model introduced by this change set.Also applies to: 131-131
tests/unit/observability/test_sink_routing.py (1)
87-89: Targeted routing coverage added.This test closes the gap for the new
access.logsink by asserting the intendedsynthorg.api.prefix routing.cli/testdata/compose_custom_ports.yml (1)
18-18: Fixture update is correct.Adding
SYNTHORG_LOG_DIRhere keeps the custom-ports compose golden file in sync with backend logging bootstrap expectations.cli/testdata/compose_digest_pins.yml (1)
18-18: Digest-pinned fixture stays consistent.
SYNTHORG_LOG_DIRinclusion here matches the updated backend env contract and keeps fixture parity across compose variants.docker/compose.yml (1)
22-23: Compose wiring looks solid.The defaulted
SYNTHORG_LOG_DIRandSYNTHORG_LOG_LEVELenv mappings are correctly exposed on the backend service.src/synthorg/api/server.py (1)
58-59: Uvicorn logging handoff is correct.Disabling Uvicorn access logs and its default
log_configcleanly avoids duplicate/unstructured output now that structured logging is bootstrapped in-app.cli/internal/compose/compose.yml.tmpl (1)
18-18: Template change is appropriate.Adding
SYNTHORG_LOG_DIRin the backend environment keeps generated compose output aligned with the new logging bootstrap behavior.tests/unit/observability/test_config.py (1)
276-295: Count assertions correctly reflect the new eight-sink baseline.Line 277 and Line 295 are consistent with the added
access.logdefault sink.cli/testdata/compose_default.yml (1)
18-18: Good compose testdata alignment for configurable log directory.Line 18 correctly wires
SYNTHORG_LOG_DIRto the persistent/datamount.src/synthorg/observability/sinks.py (1)
28-34: Routing table extension foraccess.loglooks correct.Line 33 cleanly scopes
access.logtosynthorg.api.logger names via existing prefix filter behavior.src/synthorg/observability/config.py (1)
241-247:DEFAULT_SINKSaddition is consistent and correctly typed.The new
access.logSinkConfigentry (Line 241-Line 247) matches the existing default sink schema and levels.src/synthorg/observability/events/communication.py (1)
50-54: New idle-summary event constant is well-formed.Line 53 follows the domain event naming pattern and supports the throttled idle logging flow.
tests/unit/communication/test_bus_memory.py (1)
742-817: Deterministic idle-summary testing is solid here.The new tests correctly use mocked monotonic time and
timeout=0.0, which makes the idle-summary behavior predictable and non-flaky.
| # --no-access-log: Uvicorn's default access logger is disabled because | ||
| # RequestLoggingMiddleware provides richer structured access logging via | ||
| # structlog (method, path, status_code, duration_ms, request_id). | ||
| CMD ["/app/.venv/bin/uvicorn", "synthorg.api.app:create_app", "--factory", "--no-access-log"] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In current uvicorn source (uvicorn/main.py), what value does the CLI pass to run(..., log_config=...)when--log-config is not provided?
💡 Result:
When --log-config is not provided (so log_config is None), the CLI calls:
run(..., log_config=LOGGING_CONFIG) — i.e., it passes uvicorn.config.LOGGING_CONFIG. [1]
Sources: [1] (raw.githubusercontent.com)
Citations:
Disable Uvicorn's default logging config to match the structured logging pipeline.
The --no-access-log flag disables only the access logger. However, when --log-config is omitted from the CLI invocation, Uvicorn defaults to applying its standard LOGGING_CONFIG dict, which bypasses the structured logging pipeline. Either pass --log-config /dev/null to suppress it, or refactor the container startup to call uvicorn.run(log_config=None) directly to ensure all logs route through RequestLoggingMiddleware.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/backend/Dockerfile` around lines 125 - 128, The Docker CMD currently
starts Uvicorn with "--no-access-log" but still allows Uvicorn's default
LOGGING_CONFIG to be applied, bypassing RequestLoggingMiddleware; fix by either
adding the CLI flag "--log-config /dev/null" to the existing CMD that runs
"/app/.venv/bin/uvicorn synthorg.api.app:create_app --factory --no-access-log"
so the default logging config is suppressed, or refactor startup so that
synthorg.api.app calls uvicorn.run(..., log_config=None) (and remove the Docker
CMD invocation) ensuring all logs flow through RequestLoggingMiddleware.
| | Sink | Type | Level | Format | Routes | Description | | ||
| |------|------|-------|--------|--------|-------------| | ||
| | Console | stderr | INFO | Colored text | All loggers | Human-readable development output | | ||
| | `synthorg.log` | File | INFO | JSON | All loggers | Main application log (catch-all) | | ||
| | `audit.log` | File | INFO | JSON | `synthorg.security.*` | Security events only | | ||
| | `errors.log` | File | ERROR | JSON | All loggers | Errors and above only | | ||
| | `agent_activity.log` | File | DEBUG | JSON | `synthorg.engine.*`, `synthorg.core.*` | Agent execution and task lifecycle | | ||
| | `cost_usage.log` | File | INFO | JSON | `synthorg.budget.*`, `synthorg.providers.*` | Cost records and provider calls | | ||
| | `debug.log` | File | DEBUG | JSON | All loggers | Full debug trace (catch-all) | | ||
| | `access.log` | File | INFO | JSON | `synthorg.api.*` | HTTP request/response access log | |
There was a problem hiding this comment.
Narrow access.log routing before calling it an HTTP access log.
Routing synthorg.api.* into this sink will also capture non-request events like API_APP_STARTUP and API_APP_SHUTDOWN from src/synthorg/api/app.py, so access.log will not stay request-only. Use a dedicated access-logger namespace or an event filter if this file is meant to be the single structured HTTP access log.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/operations.md` around lines 1146 - 1155, The current sink routing
sends all synthorg.api.* events to access.log which also captures non-request
events like API_APP_STARTUP and API_APP_SHUTDOWN; update the routing so
access.log only receives actual HTTP access entries by either using a dedicated
logger name (e.g., synthorg.api.access or synthorg.api.http_access) in the code
that emits request logs, or add an event-level filter to exclude lifecycle
events (API_APP_STARTUP, API_APP_SHUTDOWN) from the access.log sink; change the
logger used in src/synthorg/api/app.py where startup/shutdown are emitted or
adjust the sink routes in the logging configuration to reference the new
namespace/filter so access.log remains request-only.
| # Activate the structured logging pipeline (8 sinks) before any | ||
| # other setup so that auto-wiring, persistence, and bus logs all | ||
| # flow through the configured sinks. Respects SYNTHORG_LOG_DIR | ||
| # env var for Docker log directory override. | ||
| try: | ||
| effective_config = _bootstrap_app_logging(effective_config) | ||
| except Exception as exc: |
There was a problem hiding this comment.
This still leaves observability settings startup-only.
The new docs advertise root_log_level and enable_correlation as runtime-editable, but they can only take effect here during app construction. _build_settings_dispatcher() in this module still wires only provider, memory, and backup subscribers, so SettingsService changes will not reconfigure logging after startup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/app.py` around lines 429 - 435, The app currently applies
logging settings only at startup via _bootstrap_app_logging(effective_config)
and _build_settings_dispatcher() wires only provider/memory/backup subscribers
so runtime changes to root_log_level or enable_correlation never reconfigure
logging; update _build_settings_dispatcher (or add a dedicated settings
subscriber) to subscribe the SettingsService to changes for the logging keys
(e.g., "root_log_level", "enable_correlation") and, on change, invoke a
reconfiguration path that reapplies _bootstrap_app_logging or a smaller
reconfigure_logging(effective_config) helper using the new settings (ensuring it
runs safely from the dispatcher and updates existing logger sinks/correlation
behavior without requiring process restart).
| ) | ||
| root_logger.addHandler(handler) | ||
| except OSError, RuntimeError, ValueError: | ||
| except (OSError, RuntimeError, ValueError) as exc: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "Parenthesized multi-except clauses in the touched logging setup:"
rg -nP '^\s*except\s*\([^)]*\)' src/synthorg/observability/setup.py
echo
echo "Tupleless multi-except examples already used in the touched API code:"
rg -nP '^\s*except\s+[A-Za-z_][\w.]*\s*,\s*[A-Za-z_][\w.]*' src/synthorg/api/app.pyRepository: Aureliolo/synthorg
Length of output: 408
Use the tupleless multi-except form on line 145.
Change except (OSError, RuntimeError, ValueError) as exc: to except OSError, RuntimeError, ValueError as exc:. This file currently violates the PEP 758 syntax enforced by ruff on Python 3.14, while other touched code in src/synthorg/api/app.py already follows the standard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/observability/setup.py` at line 145, Replace the tuple form of
the except clause so it uses the tupleless union syntax: change the current
"except (OSError, RuntimeError, ValueError) as exc:" to "except OSError |
RuntimeError | ValueError as exc:" (keeping the same exc variable and handling
logic) in the try/except block in src/synthorg/observability/setup.py so
Ruff/Python 3.14–compatible syntax is used.
- Add test_patched_config_preserved_on_app_state: verifies that SYNTHORG_LOG_DIR override flows through create_app into AppState - Add pytestmark with timeout(30) to test_server.py - Remove redundant pre-assertion in test_server.py - Drop redundant @pytest.mark.unit from class (pytestmark covers it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [0.3.6](v0.3.5...v0.3.6) (2026-03-19) ### Features * **cli:** add backup subcommands (backup, backup list, backup restore) ([#568](#568)) ([4c06b1d](4c06b1d)) * **engine:** implement execution loop auto-selection based on task complexity ([#567](#567)) ([5bfc2c6](5bfc2c6)) ### Bug Fixes * activate structured logging pipeline -- wire 8-sink system, integrate Uvicorn, suppress spam ([#572](#572)) ([9b6bf33](9b6bf33)) * **cli:** bump grpc-go v1.79.3 -- CVE-2026-33186 auth bypass ([#574](#574)) ([f0171c9](f0171c9)) * resolve OpenAPI schema validation warnings for union/optional fields ([#558](#558)) ([5d96b2b](5d96b2b)) ### CI/CD * bump codecov/codecov-action from 5.5.2 to 5.5.3 in the minor-and-patch group ([#571](#571)) ([267f685](267f685)) * ignore chainguard/python in Dependabot docker updates ([#575](#575)) ([1935eaa](1935eaa)) ### Maintenance * bump the major group across 1 directory with 2 updates ([#570](#570)) ([b98f82c](b98f82c)) * bump the minor-and-patch group across 2 directories with 4 updates ([#569](#569)) ([3295168](3295168)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
bootstrap_logging()intocreate_app()so the comprehensive 8-sink structured logging system actually activates at startup (previously all logs fell through to Uvicorn's default stdout handler despite the full pipeline being implemented)SYNTHORG_LOG_DIRenv var for Docker volume log directory overrideRequestLoggingMiddlewarealready provides richer structured access logs) and add dedicatedaccess.logsinkSYNTHORG_LOG_LEVELenv var to override console sink levelCloses #559, closes #560, closes #561, closes #562, closes #563
Test plan
_bootstrap_app_logging(3 SYNTHORG_LOG_DIR branches)_apply_console_level_override(5 cases: no env, valid, invalid, file-only config, no console sink)Review coverage
Pre-reviewed by 10 agents (docs-consistency, code-reviewer, python-reviewer, pr-test-analyzer, silent-failure-hunter, conventions-enforcer, logging-audit, infra-reviewer, async-concurrency-reviewer, issue-resolution-verifier). 14 findings addressed.
🤖 Generated with Claude Code