-
-
Notifications
You must be signed in to change notification settings - Fork 1
[Refactor] Single responsibility principle #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
390065c to
3ad3cd2
Compare
- Extract logging initialisation into `logging.rs` and update `lib.rs` to call the helper. - Move session lifecycle logic from `lib.rs` into a new `session.rs`, keeping function signatures untouched and re-exporting via `lib.rs`. - Update module declarations and adjust imports; verify tests.
- Create `src/runtime/mod.rs` as façade exposing `RuntimeTracer`. - **Task 3A (Team A)**: Extract activation control into `runtime/activation.rs`, exposing a small struct consumed by the tracer. - **Task 3B (Team B)**: Extract value encoding routines into `runtime/value_encoder.rs`, providing unit tests and benchmarks. - **Task 3C (Team C)**: Introduce `runtime/output_paths.rs` to encapsulate format-to-filename mapping and writer initialisation. - Integrate submodules back into `runtime/mod.rs` sequentially once individual tasks are complete; resolve merge conflicts around struct fields.
- Add `monitoring/mod.rs` hosting shared types (`EventId`, `EventSet`, `ToolId`). - Split trait and dispatcher logic into `monitoring/tracer.rs`; keep callback registration helpers near the sys.monitoring bindings. - **Task 4A (Team A)**: Port OnceLock caches and registration helpers. - **Task 4B (Team B)**: Move `Tracer` trait definition and default implementations, updating call sites in runtime tracer and tests.
- Create `session.py`, `formats.py`, and `auto_start.py` with extracted logic. - Update `api.py` to delegate to the new modules but maintain backward-compatible imports. - Adjust `__init__.py` to import from `api` and trigger optional auto-start via the new helper. - Update Python tests and examples to use the reorganised structure.
- Remove obsolete comments (e.g., `//TODO AI!` placeholders) or move them into GitHub issues. - Update documentation and diagrams to reflect the new module tree. - Re-run `just test` and linting for both Rust and Python components; capture trace artifacts to confirm unchanged output format.
3ad3cd2 to
64dd247
Compare
a7e8787 to
5e2d54e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly trusting that it makes sense , i am not very familiar with the current codebase, the design ideas/motivation seems probably good
|
I wanted to make the size of each function and each file smaller in order to reduce the necessary context for further changes. But as a side-effect I think it made the code much more readable for myself |
The purpose of the refactor was to split current code into files and functions where each file/function is responsible for a single concern. See the design docs in this commit for details. No tests were touched
File-SRP
src/logging.rsfor one-time logger initialisation and migrated tracing session lifecycle (start_tracing,stop_tracing,is_tracing,flush_tracing,ACTIVEflag) intosrc/session.rs, withsrc/lib.rsnow limited to PyO3 wiring and re-exports.src/runtime/mod.rswith focusedactivation,value_encoder, andoutput_pathssubmodules;RuntimeTracernow delegates activation gating, value encoding, and writer initialisation through the façade consumed bysession.rs.src/monitoring/mod.rsfor sys.monitoring types/caches andsrc/monitoring/tracer.rsfor the tracer trait plus callback dispatch; rewiredlib.rs,session.rs, andruntime/mod.rs, and kept a top-leveltracerre-export for API stability.formats.py,session.py, andauto_start.pymodules, trimmedapi.pyto a thin façade, and moved the environment auto-start hook into__init__.py.cargo docreflects the architecture, and re-ranjust testto confirm the refactor remains green.just test(nextest + pytest) passes with the UV cache scoped to the workspace; directcargo teststill requires CPython development symbols.Function-SRP
Stage 0 – Baseline & Guardrails
just test(Rust + Python suites) passes; captured run via the top-level recipe.examples/value_capture_all.py; outputs stored inartifacts/stage0/value-capture-json/andartifacts/stage0/value-capture-binary/.ActivationControllerbehaviour and frame traversal notes for reviewer context.Stage 1 – Session Start-Up Decomposition
session::bootstraphelpers and refactoredstart_tracingto delegate directory validation, format resolution, and program metadata collection. Tests remain green._coerce_format,_validate_trace_path, and_normalize_activation_pathhelpers; added tests covering invalid formats and conflicting paths.Stage 2 – Frame Inspection & Activation Separation
runtime::frame_inspector::capture_frameto encapsulate frame lookup, locals/globals materialisation, and reference counting;on_linenow delegates to the helper while preserving behaviour.ActivationControllerwithshould_process_event/handle_return_event, updated callbacks to rely on them, and removed direct state juggling fromRuntimeTracer.Stage 3 – Value Capture Layer
runtime::value_capture::capture_call_arguments;on_py_startnow delegates to it, keeping the function focused on orchestration while reusing frame inspectors.record_visible_scopehelper and refactoredon_lineto delegate locals/globals registration through it.Stage 4 – Return Handling & Logging Harmonisation
runtime::logging::log_eventto consolidate callback logging across start, line, and return handlers.record_return_valueinruntime::value_captureand refactoredRuntimeTracer::on_py_returnto orchestrate activation checks, logging, and value recording.Stage 5 – Cleanup & Regression Sweep
design-docs/function-level-srp-refactor-plan.mdfor contributor onboarding.just test(Rustcargo nextest+ Pythonpytest) to confirm post-cleanup parity.