Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .agents/tasks/2025/08/21-0939-codetype-interface
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,6 @@ Implement the CodeObjectWrapper as designed. Update the Tracer trait as well as
There is an issue in the current implementation. We don't use caching effectively, since we create a new CodeObjectWrapper at each callback_xxx call. We need a global cache, probably keyed by the code object id. Propose design changes and update the design documents. Don't implement the changes themselves before I approve them.
--- FOLLOW UP TASK ---
Implement the global code object registry.

--- FOLLOW UP TASK ---
I have ADHD and can't follow long verbose documentation. Rewrite the documents in 'design-docs' because now they are full of shit
77 changes: 20 additions & 57 deletions design-docs/adr/0001-file-level-single-responsibility.md
Original file line number Diff line number Diff line change
@@ -1,65 +1,28 @@
# ADR 0001: File-Level Single Responsibility Refactor

- **Status:** Proposed
- **Status:** Accepted
- **Date:** 2025-10-01
- **Deciders:** Platform / Runtime Tracing Team
- **Consulted:** Python Tooling WG, Developer Experience WG

## Context

The codetracer Python recorder crate has evolved quickly and several source files now mix unrelated concerns:
- [`src/lib.rs`](../../codetracer-python-recorder/src/lib.rs) hosts PyO3 module wiring, global logging setup, tracing session state, and filesystem validation in one place.
- [`src/runtime_tracer.rs`](../../codetracer-python-recorder/src/runtime_tracer.rs) interleaves activation gating, writer lifecycle control, PyFrame helpers, and Python value encoding logic, making it challenging to test or extend any portion independently.
- [`src/tracer.rs`](../../codetracer-python-recorder/src/tracer.rs) combines sys.monitoring shim code with the `Tracer` trait, callback registration, and global caches.
- [`codetracer_python_recorder/api.py`](../../codetracer-python-recorder/codetracer_python_recorder/api.py) mixes format constants, backend interaction, context manager ergonomics, and environment based auto-start side effects.

This violates the Single Responsibility Principle (SRP) at the file level, obscures ownership boundaries, and increases the risk of merge conflicts and regressions. Upcoming work on richer value capture and optional streaming writers will add more logic to these files unless we carve out cohesive modules now.
Big files (`src/lib.rs`, `runtime_tracer.rs`, `tracer.rs`, and `codetracer_python_recorder/api.py`) each juggle multiple jobs. Upcoming value-capture + streaming work would make that worse.

## Decision

We will reorganise both the Rust crate and supporting Python package so that each file covers a single cohesive topic and exposes a narrow interface. Concretely:
1. Restrict `src/lib.rs` to PyO3 module definition and `pub use` re-exports. Move logging configuration into `src/logging.rs` and tracing session lifecycle into `src/session.rs`.
2. Split the current runtime tracer into a `runtime` module directory with dedicated files for activation control, value encoding, and output file management. The façade in `runtime/mod.rs` will assemble these pieces and expose the existing `RuntimeTracer` API.
3. Introduce a `monitoring` module directory that separates sys.monitoring primitive bindings (`EventId`, `ToolId`, registration helpers) from the `Tracer` trait and callback dispatch logic.
4. Decompose the Python helper package by moving session state management into `session.py`, format constants and validation into `formats.py`, and environment auto-start into `auto_start.py`, while keeping public functions surfaced through `api.py` and `__init__.py`.

These changes are mechanical reorganisations—no behavioural changes are expected. Public Rust and Python APIs must remain source compatible during the refactor.
Break the crate and Python package into focused modules:
- `src/lib.rs` keeps PyO3 wiring only; logging goes to `logging.rs`, session lifecycle to `session.rs`.
- `runtime/` becomes a directory with `mod.rs`, `activation.rs`, `value_encoder.rs`, `output_paths.rs`.
- `monitoring/` holds sys.monitoring types in `mod.rs` and the `Tracer` trait/dispatcher in `tracer.rs`.
- Python package gains `session.py`, `formats.py`, `auto_start.py`; `api.py` stays as the façade.
Public APIs remain unchanged—files just move.

## Consequences

- **Positive:**
- Easier onboarding for new contributors because each file advertises a single purpose.
- Improved unit testability; e.g., Python value encoding can be tested without instantiating the full tracer.
- Lower merge conflict risk: teams can edit activation logic without touching writer code.
- Clearer extension points for upcoming streaming writer and richer metadata work.
- **Negative / Risks:**
- Temporary churn in module paths may invalidate outstanding branches; mitigation is to stage work in small, reviewable PRs.
- Developers unfamiliar with Rust module hierarchies will need guidance to update `mod` declarations and `use` paths correctly.
- Python packaging changes require careful coordination to avoid circular imports when moving auto-start logic.

## Implementation Guidelines for Junior Developers

1. **Work Incrementally.** Aim for small PRs (≤500 LOC diff) that move one responsibility at a time. After each PR run `just test` and ensure all linters stay green.
2. **Preserve APIs.** When moving functions, re-export them from their new module so that existing callers (Rust and Python) compile without modification in the same PR.
3. **Add Focused Tests.** Whenever a helper is extracted (e.g., value encoding), add or migrate unit tests that cover its edge cases.
4. **Document Moves.** Update doc comments and module-level docs to reflect the new structure. Remove outdated TODOs or convert them into follow-up issues.
5. **Coordinate on Shared Types.** When splitting `runtime_tracer.rs`, agree on ownership for shared structs (e.g., `RuntimeTracer` remains in `runtime/mod.rs`). Use `pub(crate)` to keep internals encapsulated.
6. **Python Imports.** After splitting the Python modules, ensure `__all__` in `__init__.py` continues to export the public API. Use relative imports to avoid accidental circular dependencies.
7. **Parallel Work.** Follow the sequencing from `design-docs/file-level-srp-refactor-plan.md` to know when tasks can proceed in parallel.

## Testing Strategy

- Run `just test` locally before submitting each PR.
- Add targeted Rust tests for new modules (e.g., `activation` and `value_encoder`).
- Extend Python tests to cover auto-start logic and the context manager after extraction.
- Compare trace outputs against saved fixtures to ensure refactors do not alter serialized data.

## Alternatives Considered

- **Leave the layout as-is:** rejected because it impedes planned features and increases onboarding cost.
- **Large rewrite in a single PR:** rejected due to high risk and code review burden.

## Follow-Up Actions

- After completing the refactor, update architecture diagrams in `design-docs` to match the new module structure.
- Schedule knowledge-sharing sessions for new module owners to walk through their areas.
- 👍 Easier onboarding, smaller testable units, fewer merge conflicts, clear extension points.
- 👎 Short-term churn in module paths and imports; coordinate PRs carefully.

## Notes for implementers
- Move code in small PRs, re-export functions so callers keep working.
- Add/adjust unit tests when extracting helpers.
- Update docs/comments alongside code moves.
- Follow the sequencing in the file-level SRP plan for parallel work.

## Testing
- Run `just test` after each move.
- Compare trace fixtures to ensure behaviour stays the same.
70 changes: 18 additions & 52 deletions design-docs/adr/0002-function-level-srp.md
Original file line number Diff line number Diff line change
@@ -1,61 +1,27 @@
# ADR 0002: Function-Level Single Responsibility Refactor

- **Status:** Proposed
# ADR 0002: Function-Level SRP
- **Status:** Accepted
- **Date:** 2025-10-15
- **Deciders:** Platform / Runtime Tracing Team
- **Consulted:** Python Tooling WG, Developer Experience WG

## Context

The codetracer runtime currently exposes several high-traffic functions that blend unrelated concerns, making them difficult to understand, test, and evolve.

- [`codetracer-python-recorder/src/session.rs:start_tracing`](../../codetracer-python-recorder/src/session.rs) performs logging setup, state guards, filesystem validation and creation, format parsing, Python metadata collection, tracer instantiation, and sys.monitoring installation within one 70+ line function.
- [`codetracer-python-recorder/src/runtime/mod.rs:on_py_start`](../../codetracer-python-recorder/src/runtime/mod.rs) handles activation gating, synthetic filename filtering, argument collection via unsafe PyFrame calls, error logging, and call registration in a single block.
- [`codetracer-python-recorder/src/runtime/mod.rs:on_line`](../../codetracer-python-recorder/src/runtime/mod.rs) interleaves activation checks, frame navigation, locals/globals materialisation, value encoding, variable registration, and memory hygiene for reference counted objects.
- [`codetracer-python-recorder/src/runtime/mod.rs:on_py_return`](../../codetracer-python-recorder/src/runtime/mod.rs) combines activation lifecycle management with value encoding and logging.
- [`codetracer-python-recorder/codetracer_python_recorder/session.py:start`](../../codetracer-python-recorder/codetracer_python_recorder/session.py) mixes backend state checks, path normalisation, format coercion, and PyO3 bridge calls.

These hotspots violate the Single Responsibility Principle at the function level. When we add new formats, richer activation flows, or additional capture types, we risk regressions because each modification touches fragile, monolithic code blocks.
`start_tracing`, `RuntimeTracer::on_py_start/on_line/on_py_return`, and Python`s `session.start` each cram validation, activation logic, frame poking, and logging into long blocks. That makes fixes risky.

## Decision

We will refactor high-traffic functions so that each public entry point coordinates narrowly-scoped helpers, each owning a single concern.

1. **Trace session start-up:** Introduce a `TraceSessionBootstrap` (Rust) that encapsulates directory preparation, format resolution, and program metadata gathering. `start_tracing` will delegate to helpers like `ensure_trace_directory`, `resolve_trace_format`, and `collect_program_metadata`. Python-side `start` will mirror this by delegating validation to dedicated helpers (`validate_trace_path`, `coerce_format`).
2. **Frame inspection & activation gating:** Extract frame traversal and activation decisions into dedicated helpers inside `runtime/frame_inspector.rs` and `runtime/activation.rs`. Callback methods (`on_py_start`, `on_line`, `on_py_return`) will orchestrate the helpers instead of performing raw pointer work inline.
3. **Value capture pipeline:** Move argument, locals, globals, and return value capture to a `runtime::value_capture` module that exposes high-level functions such as `capture_call_arguments(frame, code)` and `record_visible_scope(writer, frame)`. These helpers will own error handling and ensure reference counting invariants, allowing callbacks to focus on control flow.
4. **Logging and error reporting:** Concentrate logging into small, reusable functions (e.g., `log_trace_event(event_kind, code, lineno)`) so that callbacks do not perform ad hoc logging alongside functional work.
5. **Activation lifecycle:** Ensure `ActivationController` remains the single owner for activation state transitions. Callbacks will query `should_process_event` and `handle_deactivation` helpers instead of duplicating checks.

The refactor maintains public APIs but reorganises internal call graphs to keep each function focused on orchestration.
Turn those hotspots into thin coordinators that call focused helpers:
- Bootstrap helpers prep directories, formats, and metadata before `start_tracing` proceeds.
- `frame_inspector` handles unsafe frame access; `ActivationController` owns gating.
- `value_capture` records arguments, scopes, and returns.
- Logging/error helpers keep messaging consistent.
Python mirrors this with `_validate_trace_path`, `_coerce_format`, etc.
APIs stay the same for callers.

## Consequences
- 👍 Smaller functions, better unit tests, clearer error handling.
- 👎 More helper modules to navigate; moving unsafe code needs care.

- **Positive:**
- Smaller, intention-revealing functions improve readability and lower the mental load for reviewers modifying callback behaviour.
- Reusable helpers unlock targeted unit tests (e.g., for path validation or locals capture) without invoking the entire tracing stack.
- Error handling becomes consistent and auditable when concentrated in dedicated helpers.
- Future features (streaming writers, selective variable capture) can extend isolated helpers rather than modifying monoliths.
- **Negative / Risks:**
- Increased number of private helper modules/functions may introduce slight organisational overhead for newcomers.
- Extracting FFI-heavy logic requires careful lifetime management; mistakes could introduce reference leaks or double-frees.
- Interim refactors might temporarily duplicate logic until all call sites migrate to the new helpers.

## Implementation Guidelines

1. **Preserve semantics:** Validate each step with `just test` and targeted regression fixtures to ensure helper extraction does not change runtime behaviour.
2. **Guard unsafe code:** When moving PyFrame interactions, wrap unsafe blocks in documented helpers with clear preconditions and postconditions.
3. **Keep interfaces narrow:** Expose helper functions as `pub(crate)` or module-private to prevent leaking unstable APIs.
4. **Add focused tests:** Unit test helpers for error cases (e.g., invalid path, unsupported format, missing frame) and integrate them into existing test suites.
5. **Stage changes:** Land extractions in small PRs, updating the surrounding code incrementally to avoid giant rewrites.
6. **Document intent:** Update docstrings and module-level docs to describe helper responsibilities, keeping comments aligned with SRP boundaries.

## Alternatives Considered

- **Status quo:** Rejected; expanding functionality would keep bloating already-complex functions.
- **Entirely new tracer abstraction:** Unnecessary; existing `RuntimeTracer` shape is viable once responsibilities are modularised.

## Follow-Up
## Guidance
- Extract one concern at a time, keep helpers `pub(crate)` when possible.
- Wrap unsafe code in documented helpers and add unit tests for each new module.
- Run `just test` + fixture comparisons after every extraction.

- Align sequencing with `design-docs/function-level-srp-refactor-plan.md`.
- Revisit performance benchmarks after extraction to ensure added indirection does not materially affect tracing overhead.
## Follow up
- Track work in the function-level SRP plan and watch performance to ensure the extra indirection stays cheap.
Loading
Loading