feat: implement enterprise logging system with structlog#73
Conversation
Add the `ai_company.observability` module providing structured logging infrastructure for the entire framework. Uses structlog with stdlib bridge to capture both application and third-party library output. Key components: - Frozen Pydantic config models (LogConfig, SinkConfig, RotationConfig) - 7 default sinks (console + 6 file: main, audit, error, agent, cost, debug) - Sensitive field sanitization processor (password, token, secret, etc.) - Correlation ID system via contextvars (request_id, task_id, agent_id) - Idempotent configure_logging() with 12 named logger levels - get_logger() convenience wrapper for consistent logger creation Also fixes: - CostRecordFactory defaults to prevent random validator violations - Adds logs/ to .gitignore - Adds S105 (hardcoded passwords) to test per-file-ignores 104 new tests, 100% module coverage.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new observability package providing structured logging: enums, Pydantic configs for sinks/rotation, a logger factory, sanitization processor, correlation-ID utilities, handler builders, an idempotent configure_logging entry point, tests/fixtures, and small project configuration updates. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,240,255,0.5)
participant App as Application
end
rect rgba(200,255,200,0.5)
participant Setup as configure_logging()
participant Struct as structlog
participant Root as stdlib root logger
participant FS as Filesystem
end
App->>Setup: call configure_logging(config)
Setup->>Root: _clear_root_handlers()
Setup->>Struct: _configure_structlog(enable_correlation)
Setup->>Setup: _build_shared_processors()
Setup->>FS: ensure log directories exist (per sink)
Setup->>Setup: for each sink → build_handler(sink, log_dir, foreign_pre_chain)
Setup->>Root: attach handler(s) to root logger
Setup->>Setup: _apply_logger_levels(config)
Setup-->>App: return (logging configured)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 establishes a robust and scalable enterprise logging system using Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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/ai_company/observability/correlation.py`:
- Around line 112-131: The decorator silently breaks for async functions because
the current synchronous wrapper in decorator will bind/unbind before the
coroutine runs; modify the with_correlation decorator to detect coroutine
functions using asyncio.iscoroutinefunction(func) and either raise a clear
TypeError when a coroutine is passed, or add an async variant: if coroutine,
create an async def wrapper that calls bind_correlation_id(request_id, task_id,
agent_id), awaits func(*args, **kwargs), and in a finally block calls
unbind_correlation_id(request_id is not None, task_id is not None, agent_id is
not None); keep the existing sync wrapper for normal functions and reference the
decorator/wrapper, bind_correlation_id and unbind_correlation_id symbols when
implementing the change.
In `@src/ai_company/observability/processors.py`:
- Around line 9-12: Update the _SENSITIVE_PATTERN regex to include additional
sensitive keywords: private_key, access_token, refresh_token, bearer, and
session so logs redact more fields; locate the _SENSITIVE_PATTERN definition and
extend the alternation list inside re.compile(...) (keeping re.IGNORECASE) to
include these new tokens (e.g., add
private_key|access_token|refresh_token|bearer|session) while preserving the
existing patterns and flags.
In `@src/ai_company/observability/setup.py`:
- Around line 55-120: The configure_logging function is too long (~66 lines);
split it into smaller helpers to keep each function under 50 lines: extract the
structlog configuration block into a helper
_configure_structlog(shared_processors: list) that calls structlog.configure and
sets logger_factory/wrapper_class/cache options, move the root handler clearing
and root logger level setting into _clear_root_handlers() which resets structlog
and removes/closes existing handlers then sets root level to DEBUG, move the
per-sink handler creation into _attach_handlers(config: LogConfig,
shared_processors: list) which iterates config.sinks and calls
build_handler(...) and adds handlers to the root logger, and factor out the
logger-level application into _apply_logger_levels(config: LogConfig) which
applies _DEFAULT_LOGGER_LEVELS and config.logger_levels; update
configure_logging to call _clear_root_handlers(), _build_shared_processors(),
_configure_structlog(...), _attach_handlers(...), and _apply_logger_levels(...)
in the existing order.
- Around line 78-79: Replace the hardcoded logging.DEBUG assignment with the
configured root level: use the LogConfig instance's root_level
(config.root_level) and convert it to the stdlib level before calling
root_logger.setLevel(...). In other words, locate where
root_logger.setLevel(logging.DEBUG) is called in setup.py and change it to set
the level from config.root_level (via the existing enum-to-int mapping or a
small helper that maps LogLevel values to logging.* integers) so the root logger
respects the configured LogConfig.root_level.
In `@src/ai_company/observability/sinks.py`:
- Around line 16-17: The import of Path is currently guarded by TYPE_CHECKING
but Path is used at runtime (e.g., expression log_dir / sink.file_path), causing
NameError; move the Path import out of the TYPE_CHECKING block to a normal
module-level import (remove it from inside the TYPE_CHECKING conditional) so
Path is available at runtime wherever functions or classes in this module
reference it.
- Around line 26-87: The function build_handler is over the 50-line limit;
extract the file-sink-specific creation logic into one or two small helpers
(e.g. _build_file_handler and/or _build_rotation_handler) that handle: asserting
sink.file_path, creating parent dirs, resolving rotation = sink.rotation or
RotationConfig(), and returning either a RotatingFileHandler
(RotationStrategy.BUILTIN) or WatchedFileHandler otherwise; keep the console
branch (SinkType.CONSOLE) in build_handler, preserve setting
handler.setLevel(sink.level.value) and attaching ProcessorFormatter (with
foreign_pre_chain and chosen renderer from sink.json_format → JSONRenderer or
ConsoleRenderer), and have build_handler call the new helper(s) to reduce its
line count.
In `@tests/unit/observability/conftest.py`:
- Around line 8-10: Move the type import out of the TYPE_CHECKING block so
runtime and static typing imports are consistent: take the "from collections.abc
import Iterator" import currently guarded by TYPE_CHECKING and place it at
module top-level alongside the runtime "import structlog" (keep structlog as a
runtime import), so TYPE_CHECKING is no longer the only import block and type
annotations that reference Iterator remain valid.
In `@tests/unit/observability/test_config.py`:
- Around line 107-109: The test name and expectation are confusing because the
validator for SinkConfig.file_path currently raises "whitespace-only" for both
empty and all-whitespace inputs; update the validator in config.py that
validates SinkConfig.file_path to raise a clearer message such as "file_path
must not be empty or whitespace-only" (reference the SinkConfig class and its
file_path validator), and update the unit test test_file_sink_rejects_empty_path
to expect/match that new message instead of "whitespace-only".
In `@tests/unit/observability/test_logger.py`:
- Around line 29-32: The test test_initial_bindings_applied only checks the
logger exists but not that service="api" was applied; update it to emit a log
via get_logger("test.bindings", service="api") (after calling
configure_logging()) and capture the emitted record (e.g., using pytest's caplog
or a temporary handler) then assert the captured record contains the binding
service == "api" (or that the serialized log output includes "service":"api");
reference configure_logging and get_logger when locating the test and add the
capture+assert steps to strengthen the assertion.
In `@tests/unit/observability/test_setup.py`:
- Around line 65-68: Update tests to assert that configure_logging respects a
custom root level by adding a new test (e.g., test_custom_root_level_applied)
that creates a LogConfig with root_level set to LogLevel.WARNING (using
_console_only_config().sinks), calls configure_logging(config), then checks
logging.getLogger().level equals logging.WARNING; keep the existing
test_root_logger_set_to_debug but add this new test to validate custom
root_level behavior for the configure_logging path.
In `@tests/unit/observability/test_sinks.py`:
- Around line 42-64: The four console handler tests
(test_returns_stream_handler, test_has_processor_formatter,
test_console_json_format, test_handler_level_matches_config) should accept the
handler_cleanup fixture to ensure StreamHandler is closed; update each test
signature to add the handler_cleanup parameter and run existing assertions
unchanged (these tests call build_handler, SinkConfig, SinkType, LogLevel and
assert ProcessorFormatter/type/level) so the only change is adding the
handler_cleanup fixture to each test function parameter list.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.gitignorepyproject.tomlsrc/ai_company/observability/__init__.pysrc/ai_company/observability/_logger.pysrc/ai_company/observability/config.pysrc/ai_company/observability/correlation.pysrc/ai_company/observability/enums.pysrc/ai_company/observability/processors.pysrc/ai_company/observability/setup.pysrc/ai_company/observability/sinks.pytests/unit/budget/conftest.pytests/unit/observability/__init__.pytests/unit/observability/conftest.pytests/unit/observability/test_config.pytests/unit/observability/test_correlation.pytests/unit/observability/test_enums.pytests/unit/observability/test_logger.pytests/unit/observability/test_processors.pytests/unit/observability/test_setup.pytests/unit/observability/test_sinks.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). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations
Include type hints on all public functions in Python
Use Google-style docstrings on all public classes and functions in Python
Create new objects instead of mutating existing ones — maintain immutability
Use Pydantic v2BaseModel,model_validator, andConfigDictfor data models
Enforce 88-character line length in Python code
Keep functions under 50 lines of code
Keep Python files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries — user input, external APIs, config files
Files:
src/ai_company/observability/processors.pytests/unit/observability/test_config.pytests/unit/observability/test_correlation.pysrc/ai_company/observability/_logger.pytests/unit/observability/test_enums.pysrc/ai_company/observability/sinks.pytests/unit/observability/test_processors.pysrc/ai_company/observability/config.pytests/unit/observability/test_setup.pytests/unit/budget/conftest.pysrc/ai_company/observability/__init__.pysrc/ai_company/observability/enums.pytests/unit/observability/test_logger.pytests/unit/observability/test_sinks.pytests/unit/observability/conftest.pysrc/ai_company/observability/correlation.pysrc/ai_company/observability/setup.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Maintain minimum 80% code coverage across test suite
Runmypyin strict mode on all source Python files for type checking
Files:
src/ai_company/observability/processors.pysrc/ai_company/observability/_logger.pysrc/ai_company/observability/sinks.pysrc/ai_company/observability/config.pysrc/ai_company/observability/__init__.pysrc/ai_company/observability/enums.pysrc/ai_company/observability/correlation.pysrc/ai_company/observability/setup.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Run
ruff checkandruff formaton all source and test Python files
Files:
src/ai_company/observability/processors.pytests/unit/observability/test_config.pytests/unit/observability/test_correlation.pysrc/ai_company/observability/_logger.pytests/unit/observability/test_enums.pysrc/ai_company/observability/sinks.pytests/unit/observability/test_processors.pysrc/ai_company/observability/config.pytests/unit/observability/test_setup.pytests/unit/budget/conftest.pysrc/ai_company/observability/__init__.pysrc/ai_company/observability/enums.pytests/unit/observability/test_logger.pytests/unit/observability/test_sinks.pytests/unit/observability/conftest.pysrc/ai_company/observability/correlation.pysrc/ai_company/observability/setup.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers on test functions
Set test timeout to 30 seconds per test
Files:
tests/unit/observability/test_config.pytests/unit/observability/test_correlation.pytests/unit/observability/test_enums.pytests/unit/observability/test_processors.pytests/unit/observability/test_setup.pytests/unit/budget/conftest.pytests/unit/observability/test_logger.pytests/unit/observability/test_sinks.pytests/unit/observability/conftest.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Pin all dependency versions using==operator inpyproject.toml
Organize dependencies into groups:test,dev(includes test + linting/type-checking tools) withuv syncinstalling all
Files:
pyproject.toml
🧠 Learnings (7)
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Test files must be named `test_*.py`
Applied to files:
pyproject.toml
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: Applies to tests/**/*.py : Tests must use fake model names (e.g., `test-model:8b`, `fake-writer:latest`)—never use real model IDs from `RECOMMENDED_MODELS`.
Applied to files:
pyproject.toml
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/config/**/*.py : Use YAML for company configuration loading and validation in the `config/` module
Applied to files:
src/ai_company/observability/config.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/tests/conftest.py : Place shared pytest fixtures in `tests/conftest.py`
Applied to files:
tests/unit/observability/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Use appropriate fixture scopes (`function`, `class`, `module`, `session`) and document complex fixtures with docstrings
Applied to files:
tests/unit/observability/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Each test should be independent and not rely on other tests; use pytest fixtures for test setup (shared fixtures in `tests/conftest.py`); clean up resources in teardown/fixtures
Applied to files:
tests/unit/observability/conftest.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to tests/**/*.py : Use pytest fixtures for test setup. Shared fixtures should be in `tests/conftest.py`
Applied to files:
tests/unit/observability/conftest.py
🧬 Code graph analysis (10)
tests/unit/observability/test_correlation.py (1)
src/ai_company/observability/correlation.py (5)
bind_correlation_id(36-60)clear_correlation_ids(87-89)generate_correlation_id(27-33)unbind_correlation_id(63-84)with_correlation(92-131)
tests/unit/observability/test_enums.py (1)
src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)
src/ai_company/observability/sinks.py (2)
src/ai_company/observability/config.py (2)
RotationConfig(22-46)SinkConfig(49-92)src/ai_company/observability/enums.py (2)
RotationStrategy(20-29)SinkType(32-41)
tests/unit/observability/test_processors.py (1)
src/ai_company/observability/processors.py (1)
sanitize_sensitive_fields(17-38)
src/ai_company/observability/config.py (1)
src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)
tests/unit/observability/test_setup.py (3)
src/ai_company/observability/config.py (2)
LogConfig(95-168)SinkConfig(49-92)src/ai_company/observability/enums.py (2)
LogLevel(6-17)SinkType(32-41)src/ai_company/observability/setup.py (1)
configure_logging(55-120)
src/ai_company/observability/__init__.py (6)
src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/observability/config.py (3)
LogConfig(95-168)RotationConfig(22-46)SinkConfig(49-92)src/ai_company/observability/correlation.py (5)
bind_correlation_id(36-60)clear_correlation_ids(87-89)generate_correlation_id(27-33)unbind_correlation_id(63-84)with_correlation(92-131)src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)src/ai_company/observability/processors.py (1)
sanitize_sensitive_fields(17-38)src/ai_company/observability/setup.py (1)
configure_logging(55-120)
tests/unit/observability/test_logger.py (2)
src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/observability/setup.py (1)
configure_logging(55-120)
tests/unit/observability/test_sinks.py (3)
src/ai_company/observability/config.py (2)
RotationConfig(22-46)SinkConfig(49-92)src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)src/ai_company/observability/sinks.py (1)
build_handler(26-87)
src/ai_company/observability/setup.py (4)
src/ai_company/observability/config.py (1)
LogConfig(95-168)src/ai_company/observability/enums.py (1)
LogLevel(6-17)src/ai_company/observability/processors.py (1)
sanitize_sensitive_fields(17-38)src/ai_company/observability/sinks.py (1)
build_handler(26-87)
🔇 Additional comments (24)
tests/unit/budget/conftest.py (1)
68-72: LGTM!The factory defaults are sensible values for test data generation. The pattern is consistent with other factories in this file (e.g.,
BudgetAlertConfigFactorywithwarn_at,critical_at, etc.), and the values (1000 input tokens, 500 output tokens, $0.05 cost) provide reasonable baseline data for budget-related tests..gitignore (1)
34-36: LGTM!Appropriate addition to ignore generated log files from the new observability package.
pyproject.toml (2)
112-112: LGTM!Adding
S105(hardcoded password detection) to test file ignores is appropriate since the test suite needs to verify sensitive field sanitization with literal test credentials.
17-17: Structlog 25.5.0 is a valid released version on PyPI (released October 27, 2025).The pinned dependency
structlog==25.5.0correctly follows the version pinning guideline using the==operator.src/ai_company/observability/_logger.py (1)
8-28: LGTM!Clean wrapper with proper type hints, Google-style docstring, and usage example. The
type: ignore[no-any-return]is acceptable given structlog's dynamic typing nature.src/ai_company/observability/processors.py (1)
17-38: LGTM!Clean implementation that correctly preserves immutability by returning a new dict. The structlog processor signature is properly followed, and the ARG001 noqa comments appropriately document the unused-but-required parameters.
src/ai_company/observability/correlation.py (2)
27-33: LGTM!Simple and correct UUID4 generation with proper type hints and docstring.
36-60: LGTM!Clean implementation that only binds non-None values and leaves existing bindings unchanged. Good use of keyword-only arguments for clarity.
src/ai_company/observability/enums.py (1)
6-41: LGTM!Clean
StrEnumdefinitions with clear docstrings. TheLogLevelvalues correctly match Python's stdlib logging level names for seamless integration.tests/unit/observability/test_enums.py (1)
10-71: LGTM!Comprehensive enum tests with proper
@pytest.mark.unitmarkers and 30-second timeout. Good coverage of member existence, string values, andstrsubclass behavior.tests/unit/observability/test_processors.py (1)
10-82: LGTM!Thorough test coverage for the sanitization processor including edge cases (empty dict, case insensitivity, multiple sensitive fields) and immutability verification. Proper use of
@pytest.mark.unitmarker and timeout.tests/unit/observability/test_logger.py (1)
1-39: LGTM!The test file follows coding guidelines:
@pytest.mark.unitmarker on the class, 30-second timeout viapytestmark, proper type hints, and clean test structure. Tests cover the essentialget_loggerbehavior including usability, name binding, initial bindings, and logger identity.tests/unit/observability/test_correlation.py (1)
1-176: LGTM!Excellent test coverage for correlation ID management. Tests cover all key scenarios: UUID generation/validation, binding/unbinding individual and multiple IDs, clearing context, decorator lifecycle (including exception handling), and
functools.wrapspreservation. Proper use of@pytest.mark.unitand timeout markers.tests/unit/observability/test_sinks.py (1)
1-172: LGTM!Thorough test coverage for
build_handleracross console and file sinks, BUILTIN and EXTERNAL rotation strategies. Thehandler_cleanupfixture properly manages file handler lifecycle. Tests verify handler types, levels, formatters, directory creation, and rotation parameters.tests/unit/observability/test_config.py (1)
1-253: LGTM!Comprehensive test coverage for all configuration models: defaults, custom values, validation errors, immutability (frozen), factory usage, and JSON round-trips. Tests properly verify edge cases like duplicate logger names, duplicate file paths, and blank log_dir rejection. Well-organized with clear test class separation.
src/ai_company/observability/setup.py (1)
1-120: Well-structured logging setup with clear step-by-step comments.The implementation correctly resets state for idempotency, builds processor chains, and wires structlog with stdlib. The sanitization processor integration and per-logger level overrides work as intended.
tests/unit/observability/test_setup.py (1)
1-150: LGTM!Excellent test coverage for
configure_logging: default/custom configurations, idempotency verification, per-logger level overrides, file output with JSON parsing, and stdlib bridge integration. Tests are well-organized and use appropriate helper functions for config creation.tests/unit/observability/conftest.py (2)
51-61: Good autouse fixture for test isolation.The
_reset_loggingfixture properly cleans up structlog and stdlib logging state after each test, ensuring test independence. The defaultscope="function"is correct for this use case.Based on learnings: "Each test should be independent and not rely on other tests; use pytest fixtures for test setup".
19-45: LGTM!Well-defined factories using
polyfactory.ModelFactorywith sensible defaults matching the production config models.LogConfigFactorycorrectly includes a valid sink to satisfy the "at least one sink" validator.src/ai_company/observability/__init__.py (1)
1-53: Clean public API surface.The module correctly re-exports all necessary components from submodules with a well-organized
__all__. The alphabetical ordering aids discoverability. The module docstring clearly explains the intended usage pattern (callconfigure_loggingat startup, useget_loggerelsewhere).src/ai_company/observability/config.py (4)
22-46: Well-designed rotation configuration.The model correctly enforces immutability via
frozen=True, applies appropriate field constraints (gt=0for max_bytes,ge=0for backup_count), and provides sensible defaults matching the spec (10 MB rotation size, 5 backup files).
49-92: Solid sink configuration with appropriate validation.The
_validate_file_sink_requires_pathvalidator correctly enforces that FILE sinks must have a non-blankfile_pathwhile allowing CONSOLE sinks to omit it. The validation checks bothNoneand whitespace-only strings, covering edge cases properly.
95-168: Comprehensive validation logic.The validators correctly handle edge cases:
_validate_no_duplicate_file_pathsproperly filters for FILE sinks only (line 153)- Duplicate detection uses
Counterefficiently- Error messages include the problematic values for debugging
One design consideration: the
sinksfield is required with no default, meaningLogConfig()cannot be instantiated without explicitly passing sinks. This forces explicit configuration, which is a valid pattern, though you might considerdefault=DEFAULT_SINKSif you want a zero-argument instantiation path.
171-219: Well-structured default sink configuration.The seven-sink layout matches the design spec: one colored console sink plus six purpose-specific JSON file sinks. The level assignments are appropriate (ERROR for errors.log, DEBUG for agent_activity.log and debug.log, INFO for the rest). All file sinks correctly include rotation configuration.
| _SENSITIVE_PATTERN: re.Pattern[str] = re.compile( | ||
| r"(password|secret|token|api_key|api_secret|authorization|credential)", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extending the sensitive pattern for additional security coverage.
The current pattern covers common cases well, but consider adding patterns for private_key, access_token, refresh_token, bearer, and session to catch additional sensitive fields that may appear in log contexts.
💡 Suggested pattern extension
_SENSITIVE_PATTERN: re.Pattern[str] = re.compile(
- r"(password|secret|token|api_key|api_secret|authorization|credential)",
+ r"(password|secret|token|api_key|api_secret|authorization|credential|private_key|bearer|session)",
re.IGNORECASE,
)📝 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.
| _SENSITIVE_PATTERN: re.Pattern[str] = re.compile( | |
| r"(password|secret|token|api_key|api_secret|authorization|credential)", | |
| re.IGNORECASE, | |
| ) | |
| _SENSITIVE_PATTERN: re.Pattern[str] = re.compile( | |
| r"(password|secret|token|api_key|api_secret|authorization|credential|private_key|bearer|session)", | |
| re.IGNORECASE, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/observability/processors.py` around lines 9 - 12, Update the
_SENSITIVE_PATTERN regex to include additional sensitive keywords: private_key,
access_token, refresh_token, bearer, and session so logs redact more fields;
locate the _SENSITIVE_PATTERN definition and extend the alternation list inside
re.compile(...) (keeping re.IGNORECASE) to include these new tokens (e.g., add
private_key|access_token|refresh_token|bearer|session) while preserving the
existing patterns and flags.
| if TYPE_CHECKING: | ||
| from pathlib import Path |
There was a problem hiding this comment.
Path import could be at module level.
Path is used at runtime (line 57: log_dir / sink.file_path), not just for type hints. Move it out of TYPE_CHECKING to avoid a NameError at runtime.
🐛 Fix runtime import
-from typing import TYPE_CHECKING, Any
+from pathlib import Path
+from typing import Any
-if TYPE_CHECKING:
- from pathlib import Path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/observability/sinks.py` around lines 16 - 17, The import of
Path is currently guarded by TYPE_CHECKING but Path is used at runtime (e.g.,
expression log_dir / sink.file_path), causing NameError; move the Path import
out of the TYPE_CHECKING block to a normal module-level import (remove it from
inside the TYPE_CHECKING conditional) so Path is available at runtime wherever
functions or classes in this module reference it.
| if TYPE_CHECKING: | ||
| from collections.abc import Iterator | ||
| import structlog |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Move structlog import inside or outside TYPE_CHECKING consistently.
Iterator is inside TYPE_CHECKING but structlog (line 10) is outside despite being used for runtime calls. This is correct since structlog is needed at runtime in the fixture, but consider moving the Iterator import outside since it's a minor optimization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/observability/conftest.py` around lines 8 - 10, Move the type
import out of the TYPE_CHECKING block so runtime and static typing imports are
consistent: take the "from collections.abc import Iterator" import currently
guarded by TYPE_CHECKING and place it at module top-level alongside the runtime
"import structlog" (keep structlog as a runtime import), so TYPE_CHECKING is no
longer the only import block and type annotations that reference Iterator remain
valid.
| def test_file_sink_rejects_empty_path(self) -> None: | ||
| with pytest.raises(ValidationError, match="whitespace-only"): | ||
| SinkConfig(sink_type=SinkType.FILE, file_path="") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test expectation matches implementation but message is semantically misleading.
An empty string "" isn't strictly "whitespace-only" — it's empty. The test expects match="whitespace-only" which passes because the validator in config.py uses the same message for both cases. Consider updating the validator to use a clearer message like "file_path must not be empty or whitespace-only".
This is a minor semantic nit. The test correctly reflects current behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/observability/test_config.py` around lines 107 - 109, The test
name and expectation are confusing because the validator for
SinkConfig.file_path currently raises "whitespace-only" for both empty and
all-whitespace inputs; update the validator in config.py that validates
SinkConfig.file_path to raise a clearer message such as "file_path must not be
empty or whitespace-only" (reference the SinkConfig class and its file_path
validator), and update the unit test test_file_sink_rejects_empty_path to
expect/match that new message instead of "whitespace-only".
There was a problem hiding this comment.
Pull request overview
Adds a new ai_company.observability package that establishes the project’s structured logging foundation (structlog + stdlib bridge), along with comprehensive unit tests and supporting dependency/config updates.
Changes:
- Introduce
ai_company.observability(config models, sink/handler factory, processors, correlation helpers, and an idempotentconfigure_logging()entry point). - Add a full unit-test suite for the new observability module (configs, sinks, setup behavior, correlation, processors, logger wrapper).
- Update dependencies and repo hygiene (
structlogdependency, ignorelogs/, relax Ruff S105 in tests; fix a flakyCostRecordFactorydefault).
Reviewed changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Adds locked structlog dependency metadata. |
pyproject.toml |
Adds structlog==25.5.0; adds Ruff per-file ignore S105 for tests. |
.gitignore |
Ignores logs/ directory. |
tests/unit/budget/conftest.py |
Fixes CostRecordFactory defaults to satisfy validators consistently. |
src/ai_company/observability/enums.py |
Introduces logging-related StrEnum types. |
src/ai_company/observability/config.py |
Adds frozen Pydantic config models + DEFAULT_SINKS. |
src/ai_company/observability/processors.py |
Adds sensitive-field redaction processor. |
src/ai_company/observability/correlation.py |
Adds contextvars-based correlation ID helpers + decorator. |
src/ai_company/observability/sinks.py |
Implements stdlib handler factory with ProcessorFormatter integration. |
src/ai_company/observability/setup.py |
Implements idempotent structlog + stdlib bridge configuration and default per-logger levels. |
src/ai_company/observability/_logger.py |
Adds get_logger() convenience wrapper. |
src/ai_company/observability/__init__.py |
Re-exports public observability/logging API. |
tests/unit/observability/conftest.py |
Adds factories and an autouse fixture to reset logging/structlog state. |
tests/unit/observability/test_enums.py |
Tests enum behavior. |
tests/unit/observability/test_config.py |
Tests config model defaults/validation and DEFAULT_SINKS. |
tests/unit/observability/test_processors.py |
Tests redaction behavior for sensitive fields. |
tests/unit/observability/test_correlation.py |
Tests correlation ID binding/unbinding/clearing + decorator. |
tests/unit/observability/test_sinks.py |
Tests handler construction for console/file sinks and rotation strategies. |
tests/unit/observability/test_setup.py |
Tests configure_logging() behavior (handlers, idempotency, stdlib bridge, defaults). |
tests/unit/observability/test_logger.py |
Tests get_logger() wrapper behavior. |
tests/unit/observability/__init__.py |
Adds package marker for the observability test suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 3. Set root logger to DEBUG so handlers can filter individually | ||
| root_logger.setLevel(logging.DEBUG) |
There was a problem hiding this comment.
LogConfig.root_level is currently ignored: configure_logging() always sets the root logger to DEBUG. This makes the public config field misleading and prevents callers from actually raising/lowering the global threshold; consider setting the root level from config.root_level (and relying on handler levels for additional filtering) or removing the field if it’s intentionally unused.
| # 3. Set root logger to DEBUG so handlers can filter individually | |
| root_logger.setLevel(logging.DEBUG) | |
| # 3. Set root logger level from config so handlers can filter individually | |
| root_logger.setLevel(config.root_level.value) |
| structlog.configure( | ||
| processors=[ | ||
| structlog.contextvars.merge_contextvars, | ||
| structlog.stdlib.filter_by_level, | ||
| structlog.stdlib.add_logger_name, | ||
| structlog.stdlib.add_log_level, | ||
| structlog.stdlib.PositionalArgumentsFormatter(), | ||
| structlog.processors.TimeStamper(fmt="iso", utc=True), | ||
| structlog.processors.StackInfoRenderer(), | ||
| structlog.processors.format_exc_info, | ||
| structlog.processors.UnicodeDecoder(), | ||
| sanitize_sensitive_fields, | ||
| structlog.stdlib.ProcessorFormatter.wrap_for_formatter, |
There was a problem hiding this comment.
LogConfig.enable_correlation is never consulted: contextvars are merged unconditionally in both the foreign processor chain and the structlog processor chain. Either honor this flag (e.g., omit merge_contextvars/correlation processors when disabled) or drop it from the config model to avoid a config option that has no effect.
| renderer = structlog.dev.ConsoleRenderer(colors=True) | ||
|
|
||
| formatter = ProcessorFormatter( | ||
| processors=[renderer], |
There was a problem hiding this comment.
ProcessorFormatter is configured with only the renderer. When using wrap_for_formatter, structlog adds metadata keys like _record / _from_structlog; without ProcessorFormatter.remove_processors_meta (or equivalent) those can leak into output and bloat JSON logs (and may stringify entire LogRecords). Consider adding the meta-removal processor before the renderer.
| processors=[renderer], | |
| processors=[ProcessorFormatter.remove_processors_meta, renderer], |
| # TODO(#59): Integrate LogConfig with central config/ YAML loading | ||
|
|
There was a problem hiding this comment.
The PR description says this closes #72, but the code still has an explicit TODO for YAML config integration and currently provides only a programmatic API. If #72’s acceptance criteria includes YAML/env-var overrides, async-safe handlers, rotation retention, etc., please avoid auto-closing the issue (or scope the issue/PR description accordingly) to prevent tracking gaps.
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive and well-designed structured logging system using structlog, leveraging the ai_company.observability module with clear separation of concerns and robust test coverage. However, a high-severity path traversal vulnerability was identified in the log handler factory, which could lead to arbitrary file writes if the logging configuration is sourced from untrusted input. Additionally, to improve maintainability, consider reducing code duplication in the processor chain setup within src/ai_company/observability/setup.py by extracting common processors into a shared list.
| file_path = log_dir / sink.file_path | ||
| file_path.parent.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
The build_handler function constructs log file paths by joining log_dir and sink.file_path using the / operator. In Python's pathlib, if the second argument (sink.file_path) is an absolute path, it overrides the first argument (log_dir). Furthermore, there is no validation to ensure that sink.file_path does not contain directory traversal sequences like ... This allows an attacker who can influence the logging configuration (e.g., via a YAML configuration file, as planned in TODO #59) to write log data to arbitrary locations on the filesystem.
To remediate this, you should validate that sink.file_path is a relative path and does not contain traversal sequences. Additionally, you can verify that the resolved path is within the intended log_dir.
| def _build_shared_processors() -> list[Any]: | ||
| """Build the shared processor chain for foreign (stdlib) logs. | ||
|
|
||
| Returns: | ||
| A list of structlog processors applied to stdlib-originated | ||
| log records before the final renderer. | ||
| """ | ||
| return [ | ||
| structlog.contextvars.merge_contextvars, | ||
| structlog.stdlib.add_logger_name, | ||
| structlog.stdlib.add_log_level, | ||
| structlog.stdlib.PositionalArgumentsFormatter(), | ||
| structlog.processors.TimeStamper(fmt="iso", utc=True), | ||
| structlog.processors.StackInfoRenderer(), | ||
| structlog.processors.format_exc_info, | ||
| structlog.processors.UnicodeDecoder(), | ||
| sanitize_sensitive_fields, | ||
| ] |
There was a problem hiding this comment.
The processor chain defined in _build_shared_processors is very similar to the one in configure_logging, which leads to code duplication. To improve maintainability, you could define a common list of base processors and use it in both places.
For example, you could define a _BASE_PROCESSORS list and then construct the final chains from it:
_BASE_PROCESSORS: list[Any] = [
structlog.stdlib.add_logger_name,
structlog.stdlib.add_log_level,
structlog.stdlib.PositionalArgumentsFormatter(),
structlog.processors.TimeStamper(fmt="iso", utc=True),
structlog.processors.StackInfoRenderer(),
structlog.processors.format_exc_info,
structlog.processors.UnicodeDecoder(),
sanitize_sensitive_fields,
]
def _build_shared_processors() -> list[Any]:
"""Build the shared processor chain for foreign (stdlib) logs."""
return [
structlog.contextvars.merge_contextvars,
*_BASE_PROCESSORS,
]
# Then, in configure_logging:
structlog.configure(
processors=[
structlog.contextvars.merge_contextvars,
structlog.stdlib.filter_by_level,
*_BASE_PROCESSORS,
structlog.stdlib.ProcessorFormatter.wrap_for_formatter,
],
# ...
)Implements all actionable findings from 5 local review agents (code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer, type-design-analyzer) and 3 external reviewers (CodeRabbit, Copilot, Gemini). Key fixes: - Add path traversal validation for SinkConfig.file_path (security) - Add error handling around mkdir and handler construction in sinks - Split configure_logging into focused helpers (each <50 lines) - Deduplicate processor chain via _BASE_PROCESSORS constant - Wire enable_correlation and root_level config fields - Add async function detection to with_correlation decorator - Fix clear_correlation_ids to preserve non-correlation contextvars - Add ProcessorFormatter.remove_processors_meta to formatter chain - Extend sensitive field pattern with private_key, bearer, session - Guard against non-string keys in sanitize processor - Add 12 new tests (116 total, up from 104)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive and well-structured observability module using structlog. However, critical security vulnerabilities have been identified in the validation logic. Specifically, path traversal checks are incomplete, allowing absolute paths to bypass directory restrictions, and the sensitive field redaction mechanism is shallow, making it bypassable by nested data structures or non-standard key naming. Furthermore, a critical syntax issue currently prevents the code from running. Addressing these issues is crucial to ensure the logging system is robust against malicious configuration or accidental data leakage and fully functional.
| foreign_pre_chain=shared_processors, | ||
| ) | ||
| root_logger.addHandler(handler) | ||
| except OSError, RuntimeError, ValueError: |
There was a problem hiding this comment.
This except syntax is for Python 2. For Python 3, multiple exceptions must be caught as a tuple. This will cause a SyntaxError in the target Python 3.14 environment.
| except OSError, RuntimeError, ValueError: | |
| except (OSError, RuntimeError, ValueError): |
| if ".." in PurePath(self.file_path).parts: | ||
| msg = f"file_path must not contain '..' components: {self.file_path}" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
The SinkConfig model validates file_path for .. components but fails to check for absolute paths. When constructing the full log file path in sinks.py, an absolute file_path will cause the log_dir to be ignored (e.g., Path("logs") / "/etc/passwd" results in Path("/etc/passwd")). This allows an attacker who can influence the configuration to write log files to arbitrary locations on the filesystem, potentially overwriting sensitive files or creating malicious ones.
| if ".." in PurePath(self.file_path).parts: | |
| msg = f"file_path must not contain '..' components: {self.file_path}" | |
| raise ValueError(msg) | |
| if ".." in PurePath(self.file_path).parts: | |
| msg = f"file_path must not contain '..' components: {self.file_path}" | |
| raise ValueError(msg) | |
| if PurePath(self.file_path).is_absolute() or self.file_path.startswith(("/", "\\")): | |
| msg = f"file_path must be a relative path: {self.file_path}" | |
| raise ValueError(msg) |
| def _validate_log_dir_not_blank(self) -> Self: | ||
| """Ensure ``log_dir`` is not blank.""" | ||
| if not self.log_dir.strip(): | ||
| msg = "log_dir must not be blank" | ||
| raise ValueError(msg) | ||
| return self |
There was a problem hiding this comment.
The LogConfig model does not validate the log_dir field for path traversal components such as ... An attacker who can control the log_dir configuration can specify a path that points outside the intended logging directory. This can be used to write logs to sensitive locations on the filesystem.
@model_validator(mode="after")
def _validate_log_dir_safe(self) -> Self:
"""Ensure ``log_dir`` is not blank and does not contain '..'."""
if not self.log_dir.strip():
msg = "log_dir must not be blank"
raise ValueError(msg)
if ".." in PurePath(self.log_dir).parts:
msg = f"log_dir must not contain '..' components: {self.log_dir}"
raise ValueError(msg)
return self| def sanitize_sensitive_fields( | ||
| logger: Any, # noqa: ARG001 | ||
| method_name: str, # noqa: ARG001 | ||
| event_dict: MutableMapping[str, Any], | ||
| ) -> Mapping[str, Any]: | ||
| """Redact values of keys matching sensitive patterns. | ||
|
|
||
| Returns a new dict rather than mutating the original event dict, | ||
| following the project's immutability convention. | ||
|
|
||
| Args: | ||
| logger: The wrapped logger object (unused, required by structlog). | ||
| method_name: The name of the log method called (unused). | ||
| event_dict: The event dictionary to process. | ||
|
|
||
| Returns: | ||
| A new event dict with sensitive values replaced by | ||
| ``**REDACTED**``. | ||
| """ | ||
| return { | ||
| key: ( | ||
| _REDACTED | ||
| if isinstance(key, str) and _SENSITIVE_PATTERN.search(key) | ||
| else value | ||
| ) | ||
| for key, value in event_dict.items() | ||
| } |
There was a problem hiding this comment.
The sanitize_sensitive_fields processor is designed to redact sensitive information from log entries, but it has several significant bypasses. It only checks top-level keys in the event_dict, meaning sensitive information nested within a dictionary or list will not be redacted. Additionally, it does not inspect the log message itself (the event key) and the regex used to identify sensitive keys is incomplete (e.g., it misses variations like api-key).
def sanitize_sensitive_fields(
logger: Any, # noqa: ARG001
method_name: str, # noqa: ARG001
event_dict: MutableMapping[str, Any],
) -> Mapping[str, Any]:
"""Redact values of keys matching sensitive patterns recursively."""
def _sanitize(obj: Any) -> Any:
if isinstance(obj, dict):
return {
k: ("**REDACTED**" if isinstance(k, str) and _SENSITIVE_PATTERN.search(k) else _sanitize(v))
for k, v in obj.items()
}
if isinstance(obj, list):
return [_sanitize(i) for i in obj]
return obj
return _sanitize(event_dict)There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 22 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| if TYPE_CHECKING: | ||
| from pathlib import Path | ||
|
|
There was a problem hiding this comment.
Path is imported only under TYPE_CHECKING, but it’s referenced by runtime-visible annotations in this module. With Python 3.14+ (PEP 649) lazy annotations, typing.get_type_hints() / introspection can raise NameError because Path isn’t in module globals. Import Path at runtime (optionally with # noqa: TC003) or avoid non-runtime names in annotations.
| from typing import TYPE_CHECKING, Any | |
| if TYPE_CHECKING: | |
| from pathlib import Path | |
| from typing import Any | |
| from pathlib import Path |
| if TYPE_CHECKING: | ||
| from collections.abc import Iterator | ||
| import structlog |
There was a problem hiding this comment.
Iterator is imported only under TYPE_CHECKING, but it’s referenced by a runtime-visible annotation later in this file (_reset_logging() -> Iterator[None]). With Python 3.14+ (PEP 649) lazy annotations, evaluating type hints can raise NameError because Iterator won’t be in module globals. Import Iterator at runtime (or adjust the annotation) to avoid annotation-evaluation failures.
| Attributes: | ||
| sink_type: Where to send log output (console or file). | ||
| level: Minimum log level for this sink. | ||
| file_path: Relative path for FILE sinks (within ``log_dir``). | ||
| Must be ``None`` for CONSOLE sinks, required for FILE sinks. | ||
| rotation: Rotation settings for FILE sinks. |
There was a problem hiding this comment.
The SinkConfig.file_path docstring says it “must be None for CONSOLE sinks”, but the model currently allows a file_path for console sinks (and tests assert it’s preserved). This is misleading for API consumers; either enforce file_path is None for CONSOLE sinks in validation, or update the documentation/Field description to state that file_path is ignored for console sinks.
| foreign_pre_chain=shared_processors, | ||
| ) | ||
| root_logger.addHandler(handler) | ||
| except OSError, RuntimeError, ValueError: |
There was a problem hiding this comment.
The except clause here is invalid Python syntax (except OSError, RuntimeError, ValueError:) and will raise a SyntaxError on import, preventing the logging module from being used at all. Use an exception tuple (and ideally bind the exception) e.g. except (OSError, RuntimeError, ValueError) as exc: so handler setup failures are handled as intended.
| except OSError, RuntimeError, ValueError: | |
| except (OSError, RuntimeError, ValueError): |
|
|
||
| import structlog | ||
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Callable | ||
|
|
There was a problem hiding this comment.
Callable is imported only under TYPE_CHECKING, but it’s referenced by runtime-visible type annotations in this module. With the repo’s Python 3.14+ (PEP 649) lazy-annotation setup, typing.get_type_hints() / introspection can raise NameError because Callable isn’t in module globals. Import Callable at runtime (optionally with # noqa: TC003) or otherwise avoid non-runtime names in annotations.
| import structlog | |
| if TYPE_CHECKING: | |
| from collections.abc import Callable | |
| from collections.abc import Callable # noqa: TC003 | |
| import structlog |
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Mapping, MutableMapping | ||
|
|
||
| _SENSITIVE_PATTERN: re.Pattern[str] = re.compile( | ||
| r"(password|secret|token|api_key|api_secret|authorization" | ||
| r"|credential|private_key|bearer|session)", |
There was a problem hiding this comment.
Mapping / MutableMapping are imported only under TYPE_CHECKING, but they’re used by runtime-visible annotations in this module. With Python 3.14+ (PEP 649) lazy annotations, typing.get_type_hints() / introspection can raise NameError because those names aren’t in module globals. Import them at runtime (optionally with # noqa: TC003) or avoid non-runtime names in annotations.
| from typing import TYPE_CHECKING, Any | |
| if TYPE_CHECKING: | |
| from collections.abc import Mapping, MutableMapping | |
| _SENSITIVE_PATTERN: re.Pattern[str] = re.compile( | |
| r"(password|secret|token|api_key|api_secret|authorization" | |
| r"|credential|private_key|bearer|session)", | |
| from collections.abc import Mapping, MutableMapping | |
| from typing import TYPE_CHECKING, Any | |
| _SENSITIVE_PATTERN: re.Pattern[str] = re.compile( | |
| r"(password|secret|token|api_key|api_secret|authorization" | |
| r"|credential|private_key|bearer|session)", | |
| r"(password|secret|token|api_key|api_secret|authorization" | |
| r"|credential|private_key|bearer|session)", |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/ai_company/observability/config.py`:
- Around line 89-99: Current validation only checks file_path/rotation when
self.sink_type == SinkType.FILE, allowing invalid FILE-only fields to be present
for CONSOLE sinks; update the validator in the class that uses self.sink_type
(the method referencing SinkType.FILE, self.file_path, and self.rotation) to
also reject FILE-specific fields when sink_type != SinkType.FILE by raising
ValueError if file_path is non-empty or rotation is set for any non-FILE sink,
while preserving the existing FILE-path checks (strip/.." checks) when sink_type
== SinkType.FILE.
- Around line 96-99: The current check only rejects '..' components but allows
absolute paths to escape log_dir; update the validation for self.file_path
(using PurePath/Path) to also reject absolute paths (e.g.,
PurePath(self.file_path).is_absolute()) or, better, resolve the candidate path
against self.log_dir and verify the resolved path is inside the resolved
Path(self.log_dir) (use Path(...).resolve() and
Path(...).is_relative_to/resolved.parents check) and raise ValueError with a
clear message if it would escape log_dir.
In `@src/ai_company/observability/correlation.py`:
- Around line 142-155: Replace the manual bind_correlation_id(...) / try/finally
/ contextlib.suppress(unbind_correlation_id(...)) pattern with
structlog.contextvars.bound_contextvars() so prior context is automatically
preserved/restored; specifically, wrap the call to func(...) in a with
structlog.contextvars.bound_contextvars(): block, perform
bind_correlation_id(request_id=..., task_id=..., agent_id=...) inside that
block, then call and return func(*args, **kwargs), and remove the
finalizer/unbind_correlation_id(...) and the exception suppression entirely.
In `@src/ai_company/observability/processors.py`:
- Around line 37-44: The current comprehension only redacts top-level keys in
event_dict so nested sensitive fields (e.g., payload.token) remain; replace the
comprehension with a recursive redaction helper (e.g., redact_value or
_redact_event) that walks dicts and lists: if a dict key matches
_SENSITIVE_PATTERN return _REDACTED for that key, otherwise recurse into dict
values, and for lists map the helper over each item; preserve non-container
primitives unchanged and reuse the existing symbols _REDACTED,
_SENSITIVE_PATTERN and event_dict to locate the change.
In `@src/ai_company/observability/setup.py`:
- Around line 135-140: Replace the invalid Python2-style exception clause around
the sink initialization with a parenthesized tuple and capture the exception
(e.g., use except (OSError, RuntimeError, ValueError) as e:) so the module can
import; then update the error handling in that block to optionally include the
captured exception (e) alongside the existing warning about skipping the sink to
provide explicit error context for 'sink'.
In `@tests/unit/observability/conftest.py`:
- Around line 51-61: The fixture _reset_logging currently only performs teardown
after yield, leaving stale structlog/context and stdlib logging state at test
start; change it to perform the same reset/setup before yielding as well (call
structlog.reset_defaults(), structlog.contextvars.clear_contextvars(), clear and
close root.handlers from logging.getLogger(), and set
root.setLevel(logging.WARNING) before the yield, keeping the existing post-yield
cleanup to ensure both setup and teardown are covered).
In `@tests/unit/observability/test_config.py`:
- Around line 125-131: The test test_console_sink_ignores_path is asserting that
a CONSOLE SinkConfig may keep a file_path, which violates the SinkConfig
contract; update the test to expect the validator behavior instead: either
assert that constructing SinkConfig(sink_type=SinkType.CONSOLE,
file_path="ignored.log") raises the appropriate validation error (e.g.,
ValueError/ValidationError from SinkConfig) or assert that the config
normalizes/clears file_path (e.g., cfg.file_path is None) depending on our
validator design; reference SinkConfig and SinkType.CONSOLE when making the
change so the test aligns with the intended validation behavior.
In `@tests/unit/observability/test_logger.py`:
- Around line 31-35: Update the test_logger_name_bound test to actually assert
that the requested logger name is bound to emitted records: after calling
configure_logging() and logger = get_logger("my.module"), emit a log message
(e.g., logger.info(...)) and capture log output using the test harness (caplog)
or a temporary handler, then assert that the produced LogRecord(s) have
record.name == "my.module" (and/or that the formatted message contains
"my.module") to verify name binding; reference the existing test function
test_logger_name_bound and the functions configure_logging and get_logger when
locating where to add the emit-and-assert logic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
CLAUDE.mdpyproject.tomlsrc/ai_company/observability/__init__.pysrc/ai_company/observability/config.pysrc/ai_company/observability/correlation.pysrc/ai_company/observability/processors.pysrc/ai_company/observability/setup.pysrc/ai_company/observability/sinks.pytests/unit/observability/conftest.pytests/unit/observability/test_config.pytests/unit/observability/test_correlation.pytests/unit/observability/test_logger.pytests/unit/observability/test_processors.pytests/unit/observability/test_setup.pytests/unit/observability/test_sinks.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). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations
Include type hints on all public functions in Python
Use Google-style docstrings on all public classes and functions in Python
Create new objects instead of mutating existing ones — maintain immutability
Use Pydantic v2BaseModel,model_validator, andConfigDictfor data models
Enforce 88-character line length in Python code
Keep functions under 50 lines of code
Keep Python files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries — user input, external APIs, config files
Files:
tests/unit/observability/test_config.pysrc/ai_company/observability/config.pysrc/ai_company/observability/__init__.pysrc/ai_company/observability/setup.pysrc/ai_company/observability/processors.pytests/unit/observability/conftest.pysrc/ai_company/observability/sinks.pytests/unit/observability/test_logger.pytests/unit/observability/test_processors.pytests/unit/observability/test_sinks.pytests/unit/observability/test_correlation.pysrc/ai_company/observability/correlation.pytests/unit/observability/test_setup.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers on test functions
Set test timeout to 30 seconds per test
Files:
tests/unit/observability/test_config.pytests/unit/observability/conftest.pytests/unit/observability/test_logger.pytests/unit/observability/test_processors.pytests/unit/observability/test_sinks.pytests/unit/observability/test_correlation.pytests/unit/observability/test_setup.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Run
ruff checkandruff formaton all source and test Python files
Files:
tests/unit/observability/test_config.pysrc/ai_company/observability/config.pysrc/ai_company/observability/__init__.pysrc/ai_company/observability/setup.pysrc/ai_company/observability/processors.pytests/unit/observability/conftest.pysrc/ai_company/observability/sinks.pytests/unit/observability/test_logger.pytests/unit/observability/test_processors.pytests/unit/observability/test_sinks.pytests/unit/observability/test_correlation.pysrc/ai_company/observability/correlation.pytests/unit/observability/test_setup.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Maintain minimum 80% code coverage across test suite
Runmypyin strict mode on all source Python files for type checking
Files:
src/ai_company/observability/config.pysrc/ai_company/observability/__init__.pysrc/ai_company/observability/setup.pysrc/ai_company/observability/processors.pysrc/ai_company/observability/sinks.pysrc/ai_company/observability/correlation.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Pin all dependency versions using==operator inpyproject.toml
Organize dependencies into groups:test,dev(includes test + linting/type-checking tools) withuv syncinstalling all
Files:
pyproject.toml
🧠 Learnings (16)
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/config/**/*.py : Use YAML for company configuration loading and validation in the `config/` module
Applied to files:
src/ai_company/observability/config.py
📚 Learning: 2026-02-27T18:32:19.626Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Keep functions under 50 lines of code
Applied to files:
src/ai_company/observability/setup.pysrc/ai_company/observability/sinks.py
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: Pre-commit enforces 1000-line file size limit. When files approach this limit, split into logical submodules (e.g., extract helpers into `_helpers.py`) rather than condensing code.
Applied to files:
src/ai_company/observability/setup.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to **/*.py : Line length: 100 characters (enforced by Ruff)
Applied to files:
src/ai_company/observability/setup.py
📚 Learning: 2026-02-27T18:32:19.626Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Keep Python files under 800 lines
Applied to files:
src/ai_company/observability/setup.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to **/*.py : Use logging levels: debug (routine), info (significant), warning (unexpected but recoverable), error (failures)
Applied to files:
src/ai_company/observability/setup.py
📚 Learning: 2026-01-24T09:54:45.426Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/agents.instructions.md:0-0
Timestamp: 2026-01-24T09:54:45.426Z
Learning: Applies to agents/*.py : Log all significant operations using Python logging with appropriate levels: debug for detailed info, info for successes, warning for retries, error for failures
Applied to files:
src/ai_company/observability/setup.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Use appropriate fixture scopes (`function`, `class`, `module`, `session`) and document complex fixtures with docstrings
Applied to files:
tests/unit/observability/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/tests/conftest.py : Place shared pytest fixtures in `tests/conftest.py`
Applied to files:
tests/unit/observability/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Each test should be independent and not rely on other tests; use pytest fixtures for test setup (shared fixtures in `tests/conftest.py`); clean up resources in teardown/fixtures
Applied to files:
tests/unit/observability/conftest.pytests/unit/observability/test_sinks.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to tests/**/*.py : Use pytest fixtures for test setup. Shared fixtures should be in `tests/conftest.py`
Applied to files:
tests/unit/observability/conftest.py
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: When making changes that affect architecture, services, key files, settings, or workflows, update the relevant sections of existing documentation (CLAUDE.md, README.md, etc.) to reflect those changes.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Test files must be named `test_*.py`
Applied to files:
pyproject.toml
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: Applies to tests/**/*.py : Tests must use fake model names (e.g., `test-model:8b`, `fake-writer:latest`)—never use real model IDs from `RECOMMENDED_MODELS`.
Applied to files:
pyproject.toml
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: Applies to src/ui/**/*.py : Never exclude files from coverage requirements to make tests pass. Write proper tests to achieve 100% coverage. Acceptable exclusions are: (1) `# pragma: no cover` on pure UI widget creation code, and (2) standalone utility scripts in `scripts/` not part of the main application.
Applied to files:
pyproject.toml
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Use `async def` for async test functions; pytest-asyncio is configured with `asyncio_mode = "auto"`; mark async tests with `pytest.mark.asyncio` if needed
Applied to files:
src/ai_company/observability/correlation.py
🧬 Code graph analysis (10)
tests/unit/observability/test_config.py (3)
src/ai_company/observability/config.py (3)
LogConfig(102-175)RotationConfig(23-49)SinkConfig(52-99)src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)tests/unit/observability/conftest.py (3)
LogConfigFactory(33-45)RotationConfigFactory(19-23)SinkConfigFactory(26-30)
src/ai_company/observability/config.py (1)
src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)
src/ai_company/observability/__init__.py (6)
src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/observability/config.py (3)
LogConfig(102-175)RotationConfig(23-49)SinkConfig(52-99)src/ai_company/observability/correlation.py (5)
bind_correlation_id(38-62)clear_correlation_ids(89-99)generate_correlation_id(29-35)unbind_correlation_id(65-86)with_correlation(102-159)src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)src/ai_company/observability/processors.py (1)
sanitize_sensitive_fields(18-44)src/ai_company/observability/setup.py (1)
configure_logging(159-195)
src/ai_company/observability/setup.py (4)
src/ai_company/observability/config.py (1)
LogConfig(102-175)src/ai_company/observability/enums.py (1)
LogLevel(6-17)src/ai_company/observability/processors.py (1)
sanitize_sensitive_fields(18-44)src/ai_company/observability/sinks.py (1)
build_handler(80-121)
tests/unit/observability/conftest.py (2)
src/ai_company/observability/config.py (3)
LogConfig(102-175)RotationConfig(23-49)SinkConfig(52-99)src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)
src/ai_company/observability/sinks.py (2)
src/ai_company/observability/config.py (2)
RotationConfig(23-49)SinkConfig(52-99)src/ai_company/observability/enums.py (2)
RotationStrategy(20-29)SinkType(32-41)
tests/unit/observability/test_logger.py (4)
src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/observability/config.py (2)
LogConfig(102-175)SinkConfig(52-99)src/ai_company/observability/enums.py (2)
LogLevel(6-17)SinkType(32-41)src/ai_company/observability/setup.py (1)
configure_logging(159-195)
tests/unit/observability/test_processors.py (1)
src/ai_company/observability/processors.py (1)
sanitize_sensitive_fields(18-44)
tests/unit/observability/test_sinks.py (3)
src/ai_company/observability/config.py (2)
RotationConfig(23-49)SinkConfig(52-99)src/ai_company/observability/enums.py (3)
LogLevel(6-17)RotationStrategy(20-29)SinkType(32-41)src/ai_company/observability/sinks.py (1)
build_handler(80-121)
tests/unit/observability/test_correlation.py (1)
src/ai_company/observability/correlation.py (5)
bind_correlation_id(38-62)clear_correlation_ids(89-99)generate_correlation_id(29-35)unbind_correlation_id(65-86)with_correlation(102-159)
🔇 Additional comments (8)
CLAUDE.md (1)
37-37: Documentation now reflects the new package boundary.Line 37 correctly adds
observability/to the package map, keeping architecture docs aligned with the implementation.Based on learnings: "When making changes that affect architecture, services, key files, settings, or workflows, update the relevant sections of existing documentation (CLAUDE.md, README.md, etc.) to reflect those changes."
pyproject.toml (1)
17-17:pyproject.tomlupdates look consistent and well-scoped.The dependency and test-lint configuration changes are coherent with the current project setup and do not introduce config regressions.
As per coding guidelines: "Pin all dependency versions using
==operator inpyproject.toml."Also applies to: 112-113
src/ai_company/observability/sinks.py (1)
26-78: Handler construction flow is robust and readable.The split between
_build_file_handlerandbuild_handler, plus explicit OSError wrapping, improves failure diagnostics and keeps sink wiring straightforward.Also applies to: 80-121
src/ai_company/observability/__init__.py (1)
17-51: Public observability API surface is clean and explicit.The re-export structure and
__all__list make the package entrypoint predictable for downstream imports.tests/unit/observability/test_processors.py (1)
10-103: Strong sanitizer coverage and immutability checks.Line 10 through Line 103 provide good unit coverage for redaction, case-insensitive matching, and non-mutation behavior.
tests/unit/observability/test_setup.py (1)
72-80: Good regression test for configurable root level.Line 72 through Line 80 correctly verifies
configure_logging()honorsLogConfig.root_level.tests/unit/observability/test_sinks.py (1)
29-76: Good fixture-based handler cleanup across console tests.Line 29 through Line 76 consistently tracks and closes handlers, which improves isolation and avoids resource leaks in test runs.
tests/unit/observability/test_correlation.py (1)
150-159: Exception-path unbind test is solid.Line 150 through Line 159 correctly verifies correlation keys are removed even when the wrapped function raises.
| except OSError, RuntimeError, ValueError: | ||
| print( # noqa: T201 | ||
| f"WARNING: Failed to initialise log sink " | ||
| f"{sink!r}. This sink will be skipped.", | ||
| file=sys.stderr, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/ai_company/observability/setup.py | sed -n '130,145p'Repository: Aureliolo/ai-company
Length of output: 708
🏁 Script executed:
python -m py_compile src/ai_company/observability/setup.py 2>&1Repository: Aureliolo/ai-company
Length of output: 271
🏁 Script executed:
python -c "
# Verify the correct syntax would compile
import tempfile
import os
correct_code = '''
except (OSError, RuntimeError, ValueError) as exc:
print(f\"test {exc}\")
'''
# Check if proper syntax is valid
compile(correct_code, '<string>', 'exec')
print('Proposed fix syntax is valid')
"Repository: Aureliolo/ai-company
Length of output: 258
🏁 Script executed:
python -c "
# Verify the correct syntax in proper context
correct_code = '''
try:
pass
except (OSError, RuntimeError, ValueError) as exc:
print(f\"test {exc}\")
'''
# Check if proper syntax is valid
compile(correct_code, '<string>', 'exec')
print('Proposed fix syntax is valid in proper context')
"Repository: Aureliolo/ai-company
Length of output: 110
Fix invalid multi-exception except syntax.
Line 135 uses Python 2-style exception syntax (except OSError, RuntimeError, ValueError:); this raises SyntaxError and prevents the module from importing. Change to parenthesized tuple syntax with exception capture for explicit error handling.
🐛 Proposed fix
- except OSError, RuntimeError, ValueError:
+ except (OSError, RuntimeError, ValueError) as exc:
print( # noqa: T201
f"WARNING: Failed to initialise log sink "
- f"{sink!r}. This sink will be skipped.",
+ f"{sink!r}: {exc}. This sink will be skipped.",
file=sys.stderr,
)📝 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.
| except OSError, RuntimeError, ValueError: | |
| print( # noqa: T201 | |
| f"WARNING: Failed to initialise log sink " | |
| f"{sink!r}. This sink will be skipped.", | |
| file=sys.stderr, | |
| ) | |
| except (OSError, RuntimeError, ValueError) as exc: | |
| print( # noqa: T201 | |
| f"WARNING: Failed to initialise log sink " | |
| f"{sink!r}: {exc}. This sink will be skipped.", | |
| file=sys.stderr, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/observability/setup.py` around lines 135 - 140, Replace the
invalid Python2-style exception clause around the sink initialization with a
parenthesized tuple and capture the exception (e.g., use except (OSError,
RuntimeError, ValueError) as e:) so the module can import; then update the error
handling in that block to optionally include the captured exception (e)
alongside the existing warning about skipping the sink to provide explicit error
context for 'sink'.
- Reject absolute file_path on FILE sinks (cross-platform check) - Reject file_path/rotation on CONSOLE sinks (fail-fast validation) - Validate log_dir for path traversal (..) components - Add recursive redaction for nested sensitive fields in processors - Replace manual bind/unbind with structlog bound_contextvars() - Add setup+teardown in test fixture to prevent state leakage - Strengthen weak test assertions (logger name, config contracts)
🤖 I have created a release *beep* *boop* --- ## [0.1.1](ai-company-v0.1.0...ai-company-v0.1.1) (2026-03-10) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.1.0](v0.0.0...v0.1.0) (2026-03-11) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add mandatory JWT + API key authentication ([#256](#256)) ([c279cfe](c279cfe)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable output scan response policies ([#263](#263)) ([b9907e8](b9907e8)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement AuditRepository for security audit log persistence ([#279](#279)) ([94bc29f](94bc29f)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * resolve circular imports, bump litellm, fix release tag format ([#286](#286)) ([a6659b5](a6659b5)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * bump anchore/scan-action from 6.5.1 to 7.3.2 ([#271](#271)) ([80a1c15](80a1c15)) * bump docker/build-push-action from 6.19.2 to 7.0.0 ([#273](#273)) ([dd0219e](dd0219e)) * bump docker/login-action from 3.7.0 to 4.0.0 ([#272](#272)) ([33d6238](33d6238)) * bump docker/metadata-action from 5.10.0 to 6.0.0 ([#270](#270)) ([baee04e](baee04e)) * bump docker/setup-buildx-action from 3.12.0 to 4.0.0 ([#274](#274)) ([5fc06f7](5fc06f7)) * bump sigstore/cosign-installer from 3.9.1 to 4.1.0 ([#275](#275)) ([29dd16c](29dd16c)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * **main:** release ai-company 0.1.1 ([#282](#282)) ([2f4703d](2f4703d)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Signed-off-by: Aurelio <19254254+Aureliolo@users.noreply.github.com>
… Gemini - Fix step order: tame third-party loggers before applying per-logger levels so user config overrides take precedence (#1) - Replace contextlib.suppress(Exception) with try/except + stderr warning to match _clear_root_handlers error handling pattern (#2) - Fix misleading docstring: clarify litellm attribute suppression is conditional while handler cleanup runs unconditionally (#3) - Change PR body from Closes #72 to Related to #72 (already closed by PR #73) (#4) - Add precondition to test_litellm_suppress_debug_info_enabled (#5) - Add suppress_debug_info to CLAUDE.md observability description (#6) - Add test for handler.close() failure warning to stderr (#7) - Add test for removing multiple handlers from single logger (#8) - Add autouse fixture to reset third-party logger state between tests (#9) - Strengthen test_skips_litellm_when_not_imported to verify handler and level cleanup still works (#10) - Add test for non-LiteLLM handler removal (httpx) (#11) - Update docs/design/operations.md step number and override note Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Closes #72
Implements the foundational
ai_company.observabilitymodule — structured logging infrastructure that all future modules (engine, providers, API, security) will build on.src/ai_company/observability/(8 source files)tests/unit/observability/(8 test files, 104 tests, 100% module coverage)Source files
enums.pyLogLevel,RotationStrategy,SinkType(StrEnum)config.pyRotationConfig,SinkConfig,LogConfig(frozen Pydantic) +DEFAULT_SINKSprocessors.pysanitize_sensitive_fields— redacts password/token/secret/api_key/credentialcorrelation.pybind_correlation_id,unbind_correlation_id,clear_correlation_ids,generate_correlation_id,with_correlationdecoratorsinks.pybuild_handlerfactory — console, file, rotating, watched handlerssetup.pyconfigure_logging()— idempotent entry point, structlog + stdlib bridge, 12 named logger levels_logger.pyget_logger()convenience wrapper__init__.py7 default sinks
Console (colored) + 6 JSON file sinks: main (
ai_company.log), audit (audit.log), error (errors.log), agent activity (agent_activity.log), cost/usage (cost_usage.log), debug (debug.log)Additional fixes
CostRecordFactoryintests/unit/budget/conftest.py— set proper defaults to prevent random model validator violationslogs/to.gitignoreS105(hardcoded passwords) to test per-file-ignores inpyproject.tomlTODOs embedded in code
config.py:# TODO(#59): Integrate LogConfig with central config/ YAML loadingcorrelation.py:# TODO: Add with_correlation_async() for async functions (engine/API)sinks.py:# TODO: Add logger name filters to route specific loggers to specific sinksTest plan
uv run ruff check src/ tests/— all checks passuv run mypy src/— no issues in 38 source filesuv run pytest tests/unit/observability/ -v --cov=ai_company.observability --cov-fail-under=80— 104 passed, 100% coverageuv run pytest tests/ -n auto --cov=ai_company --cov-fail-under=80— 624 passed, 99% overall coverage