diff --git a/.gitignore b/.gitignore index 71629a1..f7ca325 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,5 @@ **/target/ build *~ -.idea/ \ No newline at end of file +.idea/ +.cargo/ \ No newline at end of file diff --git a/codetracer-python-recorder/src/runtime_tracer.rs b/codetracer-python-recorder/src/runtime_tracer.rs index 81d0ccf..3599cf2 100644 --- a/codetracer-python-recorder/src/runtime_tracer.rs +++ b/codetracer-python-recorder/src/runtime_tracer.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use pyo3::prelude::*; -use pyo3::types::PyAny; +use pyo3::types::{PyAny, PyList, PyTuple, PyDict}; use runtime_tracing::{Line, TraceEventsFileFormat, TraceWriter, TypeKind, ValueRecord, NONE_VALUE}; use runtime_tracing::NonStreamingTraceWriter; @@ -77,6 +77,17 @@ impl RuntimeTracer { } } + /// Encode a Python value into a `ValueRecord` used by the trace writer. + /// + /// Canonical rules: + /// - `None` -> `NONE_VALUE` + /// - `bool` -> `Bool` + /// - `int` -> `Int` + /// - `str` -> `String` (canonical for text; do not fall back to Raw) + /// - common containers: + /// - Python `tuple` -> `Tuple` with encoded elements + /// - Python `list` -> `Sequence` with encoded elements (not a slice) + /// - any other type -> textual `Raw` via `__str__` best-effort fn encode_value<'py>(&mut self, _py: Python<'py>, v: &Bound<'py, PyAny>) -> ValueRecord { // None if v.is_none() { @@ -91,11 +102,63 @@ impl RuntimeTracer { let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Int, "Int"); return ValueRecord::Int { i, type_id: ty }; } + // Strings are encoded canonically as `String` to ensure stable tests + // and downstream processing. Falling back to `Raw` for `str` is + // not allowed. if let Ok(s) = v.extract::() { let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::String, "String"); return ValueRecord::String { text: s, type_id: ty }; } + // Python tuple -> ValueRecord::Tuple with recursively-encoded elements + if let Ok(t) = v.downcast::() { + let mut elements: Vec = Vec::with_capacity(t.len()); + for item in t.iter() { + // item: Bound + elements.push(self.encode_value(_py, &item)); + } + let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Tuple, "Tuple"); + return ValueRecord::Tuple { elements, type_id: ty }; + } + + // Python list -> ValueRecord::Sequence with recursively-encoded elements + if let Ok(l) = v.downcast::() { + let mut elements: Vec = Vec::with_capacity(l.len()); + for item in l.iter() { + elements.push(self.encode_value(_py, &item)); + } + let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Seq, "List"); + return ValueRecord::Sequence { elements, is_slice: false, type_id: ty }; + } + + // Python dict -> represent as a Sequence of (key, value) Tuples. + // Keys are expected to be strings for kwargs; for non-str keys we + // fall back to best-effort encoding of the key. + if let Ok(d) = v.downcast::() { + let seq_ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Seq, "Dict"); + let tuple_ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Tuple, "Tuple"); + let str_ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::String, "String"); + let mut elements: Vec = Vec::with_capacity(d.len()); + let items = d.items(); + for pair in items.iter() { + if let Ok(t) = pair.downcast::() { + if t.len() == 2 { + let key_obj = t.get_item(0).unwrap(); + let val_obj = t.get_item(1).unwrap(); + let key_rec = if let Ok(s) = key_obj.extract::() { + ValueRecord::String { text: s, type_id: str_ty } + } else { + self.encode_value(_py, &key_obj) + }; + let val_rec = self.encode_value(_py, &val_obj); + let pair_rec = ValueRecord::Tuple { elements: vec![key_rec, val_rec], type_id: tuple_ty }; + elements.push(pair_rec); + } + } + } + return ValueRecord::Sequence { elements, is_slice: false, type_id: seq_ty }; + } + // Fallback to Raw string representation let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Raw, "Object"); match v.str() { @@ -124,10 +187,10 @@ impl Tracer for RuntimeTracer { events_union(&[events.PY_START, events.LINE, events.PY_RETURN]) } - fn on_py_start(&mut self, py: Python<'_>, code: &CodeObjectWrapper, _offset: i32) { + fn on_py_start(&mut self, py: Python<'_>, code: &CodeObjectWrapper, _offset: i32) -> PyResult<()> { // Activate lazily if configured; ignore until then self.ensure_started(py, code); - if !self.started { return; } + if !self.started { return Ok(()); } // Trace event entry match (code.filename(py), code.qualname(py)) { (Ok(fname), Ok(qname)) => { @@ -136,8 +199,99 @@ impl Tracer for RuntimeTracer { _ => log::debug!("[RuntimeTracer] on_py_start"), } if let Ok(fid) = self.ensure_function_id(py, code) { - TraceWriter::register_call(&mut self.writer, fid, Vec::new()); + // Attempt to capture function arguments from the current frame. + // Fail fast on any error per source-code rules. + let mut args: Vec = Vec::new(); + let frame_and_args = (|| -> PyResult<()> { + // Current Python frame where the function just started executing + let sys = py.import("sys")?; + let frame = sys.getattr("_getframe")?.call1((0,))?; + let locals = frame.getattr("f_locals")?; + + // Argument names come from co_varnames in the order defined by CPython: + // [positional (pos-only + pos-or-kw)] [+ varargs] [+ kw-only] [+ kwargs] + // In CPython 3.8+ semantics, `co_argcount` is the TOTAL number of positional + // parameters (including positional-only and pos-or-keyword). Use it directly + // for the positional slice; `co_posonlyargcount` is only needed if we want to + // distinguish the two groups, which we do not here. + let argcount = code.arg_count(py)? as usize; // total positional (pos-only + pos-or-kw) + let posonly: usize = code + .as_bound(py) + .getattr("co_posonlyargcount")? + .extract()?; + let kwonly: usize = code + .as_bound(py) + .getattr("co_kwonlyargcount")? + .extract()?; + + let flags = code.flags(py)?; + const CO_VARARGS: u32 = 0x04; + const CO_VARKEYWORDS: u32 = 0x08; + + let varnames_obj = code.as_bound(py).getattr("co_varnames")?; + let varnames: Vec = varnames_obj.extract()?; + + // 1) Positional parameters (pos-only + pos-or-kw) + let mut idx = 0usize; + // `argcount` already includes positional-only parameters + let take_n = std::cmp::min(argcount, varnames.len()); + for name in varnames.iter().take(take_n) { + match locals.get_item(name) { + Ok(val) => { + let vrec = self.encode_value(py, &val); + args.push(TraceWriter::arg(&mut self.writer, name, vrec)); + } + Err(_) => {} + } + idx += 1; + } + + // 2) Varargs (*args) + if (flags & CO_VARARGS) != 0 && idx < varnames.len() { + let name = &varnames[idx]; + if let Ok(val) = locals.get_item(name) { + let vrec = self.encode_value(py, &val); + args.push(TraceWriter::arg(&mut self.writer, name, vrec)); + } + idx += 1; + } + + // 3) Keyword-only parameters + let kwonly_take = std::cmp::min(kwonly, varnames.len().saturating_sub(idx)); + for name in varnames.iter().skip(idx).take(kwonly_take) { + match locals.get_item(name) { + Ok(val) => { + let vrec = self.encode_value(py, &val); + args.push(TraceWriter::arg(&mut self.writer, name, vrec)); + } + Err(_) => {} + } + } + idx = idx.saturating_add(kwonly_take); + + // 4) Kwargs (**kwargs) + if (flags & CO_VARKEYWORDS) != 0 && idx < varnames.len() { + let name = &varnames[idx]; + if let Ok(val) = locals.get_item(name) { + let vrec = self.encode_value(py, &val); + args.push(TraceWriter::arg(&mut self.writer, name, vrec)); + } + } + Ok(()) + })(); + if let Err(e) = frame_and_args { + // Raise a clear error; do not silently continue with empty args. + let rete =Err(pyo3::exceptions::PyRuntimeError::new_err(format!( + "on_py_start: failed to capture args: {}", + e + ))); + log::debug!("error {:?}", rete); + return rete; + } + + TraceWriter::register_call(&mut self.writer, fid, args); } + Ok(()) } fn on_line(&mut self, py: Python<'_>, code: &CodeObjectWrapper, lineno: u32) { diff --git a/codetracer-python-recorder/src/tracer.rs b/codetracer-python-recorder/src/tracer.rs index 387edab..51aef83 100644 --- a/codetracer-python-recorder/src/tracer.rs +++ b/codetracer-python-recorder/src/tracer.rs @@ -190,7 +190,11 @@ pub trait Tracer: Send + Any { } /// Called at start of a Python function (frame on stack). - fn on_py_start(&mut self, _py: Python<'_>, _code: &CodeObjectWrapper, _offset: i32) {} + /// + /// Implementations should fail fast on irrecoverable conditions + /// (e.g., inability to access the current frame/locals) by + /// returning an error. + fn on_py_start(&mut self, _py: Python<'_>, _code: &CodeObjectWrapper, _offset: i32) -> PyResult<()> { Ok(()) } /// Called on resumption of a generator/coroutine (not via throw()). fn on_py_resume(&mut self, _py: Python<'_>, _code: &CodeObjectWrapper, _offset: i32) {} @@ -557,9 +561,19 @@ fn callback_branch( fn callback_py_start(py: Python<'_>, code: Bound<'_, PyCode>, instruction_offset: i32) -> PyResult<()> { if let Some(global) = GLOBAL.lock().unwrap().as_mut() { let wrapper = global.registry.get_or_insert(py, &code); - global.tracer.on_py_start(py, &wrapper, instruction_offset); + match global.tracer.on_py_start(py, &wrapper, instruction_offset) { + Ok(()) => Ok(()), + Err(err) => { + // Disable further monitoring immediately on first callback error. + // Soft-stop within this lock to avoid deadlocking on GLOBAL. + let _ = set_events(py, &global.tool, NO_EVENTS); + log::error!("Event monitoring turned off due to exception. No new events will be recorded! {}", err); + Err(err) + } + } + } else { + Ok(()) } - Ok(()) } #[pyfunction] diff --git a/codetracer-python-recorder/test/test_monitoring_events.py b/codetracer-python-recorder/test/test_monitoring_events.py index ba2e6f4..bc9535f 100644 --- a/codetracer-python-recorder/test/test_monitoring_events.py +++ b/codetracer-python-recorder/test/test_monitoring_events.py @@ -17,6 +17,8 @@ class ParsedTrace: calls: List[int] # sequence of function_id values returns: List[Dict[str, Any]] # raw Return payloads (order preserved) steps: List[Tuple[int, int]] # (path_id, line) + varnames: List[str] # index is variable_id + call_records: List[Dict[str, Any]] # raw Call payloads (order preserved) def _parse_trace(out_dir: Path) -> ParsedTrace: @@ -30,19 +32,32 @@ def _parse_trace(out_dir: Path) -> ParsedTrace: calls: List[int] = [] returns: List[Dict[str, Any]] = [] steps: List[Tuple[int, int]] = [] + varnames: List[str] = [] + call_records: List[Dict[str, Any]] = [] for item in events: if "Function" in item: functions.append(item["Function"]) elif "Call" in item: calls.append(int(item["Call"]["function_id"])) + call_records.append(item["Call"]) # keep for arg assertions + elif "VariableName" in item: + varnames.append(item["VariableName"]) # index is VariableId elif "Return" in item: returns.append(item["Return"]) # keep raw payload for value checks elif "Step" in item: s = item["Step"] steps.append((int(s["path_id"]), int(s["line"]))) - return ParsedTrace(paths=paths, functions=functions, calls=calls, returns=returns, steps=steps) + return ParsedTrace( + paths=paths, + functions=functions, + calls=calls, + returns=returns, + steps=steps, + varnames=varnames, + call_records=call_records, + ) def _write_script(tmp: Path) -> Path: @@ -125,3 +140,141 @@ def test_start_while_active_raises(tmp_path: Path) -> None: codetracer.start(out_dir, format=codetracer.TRACE_JSON) finally: codetracer.stop() + + +def test_call_arguments_recorded_on_py_start(tmp_path: Path) -> None: + # Arrange: write a simple script with a function that accepts two args + code = ( + "def foo(a, b):\n" + " return a if len(str(b)) > 0 else 0\n\n" + "if __name__ == '__main__':\n" + " foo(1, 'x')\n" + ) + script = tmp_path / "script_args.py" + script.write_text(code) + + out_dir = tmp_path / "trace_out" + out_dir.mkdir() + + session = codetracer.start(out_dir, format=codetracer.TRACE_JSON, start_on_enter=script) + try: + runpy.run_path(str(script), run_name="__main__") + finally: + codetracer.flush() + codetracer.stop() + + parsed = _parse_trace(out_dir) + + # Locate foo() function id in this script + assert str(script) in parsed.paths + script_path_id = parsed.paths.index(str(script)) + foo_fids = [i for i, f in enumerate(parsed.functions) if f["name"] == "foo" and f["path_id"] == script_path_id] + assert foo_fids, "Expected function entry for foo()" + foo_fid = foo_fids[0] + + # Find the first Call to foo() and assert it carries two args with correct names/values + foo_calls = [cr for cr in parsed.call_records if int(cr["function_id"]) == foo_fid] + assert foo_calls, "Expected a recorded call to foo()" + call = foo_calls[0] + args = call.get("args", []) + assert len(args) == 2, f"Expected 2 args, got: {args}" + + def arg_name(i: int) -> str: + return parsed.varnames[int(args[i]["variable_id"])] + + def arg_value(i: int) -> Dict[str, Any]: + return args[i]["value"] + + # Validate names and values + assert arg_name(0) == "a" + assert arg_value(0).get("kind") == "Int" and int(arg_value(0).get("i")) == 1 + assert arg_name(1) == "b" + v1 = arg_value(1) + # String encoding must be stable for Python str values. + # Enforce canonical kind String and exact text payload. + assert v1.get("kind") == "String", f"Expected String encoding for str, got: {v1}" + assert v1.get("text") == "x" + + +def test_all_argument_kinds_recorded_on_py_start(tmp_path: Path) -> None: + # Arrange: write a script with a function using all Python argument kinds + code = ( + "def g(p, /, q, *args, r, **kwargs):\n" + " # Touch values to ensure they're present in locals\n" + " return (p if p is not None else 0) + (q if q is not None else 0) + (r if r is not None else 0)\n\n" + "if __name__ == '__main__':\n" + " g(10, 20, 30, 40, r=50, k=60)\n" + ) + script = tmp_path / "script_args_kinds.py" + script.write_text(code) + + out_dir = tmp_path / "trace_out" + out_dir.mkdir() + + session = codetracer.start(out_dir, format=codetracer.TRACE_JSON, start_on_enter=script) + try: + runpy.run_path(str(script), run_name="__main__") + finally: + codetracer.flush() + codetracer.stop() + + parsed = _parse_trace(out_dir) + + # Locate g() function id + assert str(script) in parsed.paths + script_path_id = parsed.paths.index(str(script)) + g_fids = [i for i, f in enumerate(parsed.functions) if f["name"] == "g" and f["path_id"] == script_path_id] + assert g_fids, "Expected function entry for g()" + g_fid = g_fids[0] + + # Find the first Call to g() + g_calls = [cr for cr in parsed.call_records if int(cr["function_id"]) == g_fid] + assert g_calls, "Expected a recorded call to g()" + call = g_calls[0] + args = call.get("args", []) + assert args, "Expected arguments for g() call" + + # Build name -> value mapping for convenience + def arg_name(i: int) -> str: + return parsed.varnames[int(args[i]["variable_id"])] + + def arg_value(i: int) -> Dict[str, Any]: + return args[i]["value"] + + name_to_val: Dict[str, Dict[str, Any]] = {arg_name(i): arg_value(i) for i in range(len(args))} + + # Ensure we captured all kinds by name + for expected in ("p", "q", "args", "r", "kwargs"): + assert expected in name_to_val, f"Missing argument kind: {expected} in {list(name_to_val.keys())}" + + # Validate some concrete values where encoding is unambiguous + assert name_to_val["p"].get("kind") == "Int" and int(name_to_val["p"].get("i")) == 10 + assert name_to_val["q"].get("kind") == "Int" and int(name_to_val["q"].get("i")) == 20 + assert name_to_val["r"].get("kind") == "Int" and int(name_to_val["r"].get("i")) == 50 + + # Varargs may be encoded as a structured sequence or as a Raw string; accept both + varargs_val = name_to_val["args"] + kind = varargs_val.get("kind") + assert kind in ("Sequence", "Raw", "Tuple"), f"Unexpected *args encoding kind: {kind}" + if kind in ("Sequence", "Tuple"): + elems = varargs_val.get("elements") + assert isinstance(elems, list) and len(elems) == 2 + assert elems[0].get("kind") == "Int" and int(elems[0].get("i")) == 30 + assert elems[1].get("kind") == "Int" and int(elems[1].get("i")) == 40 + else: + # Raw textual fallback + r = varargs_val.get("r", "") + assert "30" in r and "40" in r + + # Kwargs must be encoded structurally as a sequence of (key, value) tuples + kwargs_val = name_to_val["kwargs"] + assert kwargs_val.get("kind") == "Sequence", f"Expected structured kwargs encoding, got: {kwargs_val}" + elements = kwargs_val.get("elements") + assert isinstance(elements, list) and len(elements) == 1, f"Expected single kwargs pair, got: {elements}" + pair = elements[0] + assert pair.get("kind") == "Tuple", f"Expected key/value tuple, got: {pair}" + kv = pair.get("elements") + assert isinstance(kv, list) and len(kv) == 2, f"Expected 2-tuple for kwargs item, got: {kv}" + key_rec, val_rec = kv[0], kv[1] + assert key_rec.get("kind") == "String" and key_rec.get("text") == "k", f"Unexpected kwargs key encoding: {key_rec}" + assert val_rec.get("kind") == "Int" and int(val_rec.get("i")) == 60, f"Unexpected kwargs value encoding: {val_rec}" diff --git a/codetracer-python-recorder/tests/print_tracer.rs b/codetracer-python-recorder/tests/print_tracer.rs index 48f8fe3..e1e6cd8 100644 --- a/codetracer-python-recorder/tests/print_tracer.rs +++ b/codetracer-python-recorder/tests/print_tracer.rs @@ -107,11 +107,12 @@ impl Tracer for CountingTracer { } } - fn on_py_start(&mut self, py: Python<'_>, code: &CodeObjectWrapper, offset: i32) { + fn on_py_start(&mut self, py: Python<'_>, code: &CodeObjectWrapper, offset: i32) -> PyResult<()> { PY_START_COUNT.fetch_add(1, Ordering::SeqCst); if let Ok(Some(line)) = code.line_for_offset(py, offset as u32) { println!("PY_START at {}", line); } + Ok(()) } fn on_py_resume(&mut self, py: Python<'_>, code: &CodeObjectWrapper, offset: i32) { diff --git a/codetracer-python-recorder/tests/test_fail_fast_on_py_start.py b/codetracer-python-recorder/tests/test_fail_fast_on_py_start.py new file mode 100644 index 0000000..8af3109 --- /dev/null +++ b/codetracer-python-recorder/tests/test_fail_fast_on_py_start.py @@ -0,0 +1,50 @@ +import runpy +import sys +from pathlib import Path + +import pytest + + +def test_fail_fast_when_frame_access_fails(tmp_path: Path): + # Import the built extension module + import codetracer_python_recorder.codetracer_python_recorder as cpr + + # Prepare a simple program that triggers a Python function call + prog = tmp_path / "prog.py" + prog.write_text( + """ +def f(): + return 1 + +f() +""" + ) + + # Monkeypatch sys._getframe to simulate a failure when capturing args + original_getframe = getattr(sys, "_getframe") + + def boom(*_args, **_kwargs): # pragma: no cover - intentionally fails + raise RuntimeError("boom: _getframe disabled") + + sys._getframe = boom # type: ignore[attr-defined] + + try: + # Start tracing; activate only for our program path so stray imports don't trigger + cpr.start_tracing(str(tmp_path), "json", activation_path=str(prog)) + + with pytest.raises(RuntimeError) as excinfo: + runpy.run_path(str(prog), run_name="__main__") + + # Ensure the error surfaced clearly and didn’t get swallowed + assert "_getframe" in str(excinfo.value) or "boom" in str(excinfo.value) + + # After the first failure, tracing must be disabled so + # subsequent Python function calls do not trigger the same error. + # Re-running the same program path should no longer raise. + ns = runpy.run_path(str(prog), run_name="__main__") + assert isinstance(ns, dict) + finally: + # Restore state + sys._getframe = original_getframe # type: ignore[attr-defined] + cpr.stop_tracing() + diff --git a/issues.md b/issues.md index 0928ffd..489cd8e 100644 --- a/issues.md +++ b/issues.md @@ -6,8 +6,21 @@ We need to record function arguments when calling a function We have a function `encode_value` which is used to convert Python objects to value records. We need to use this function to encode the function arguments. To do that we should modify the `on_py_start` hook to load the current frame and to read the function arguments from it. +### Definition of Done +- Arguments for positional and pos-or-keyword parameters are recorded on function entry using the current frame's locals. +- Values are encoded via `encode_value` and attached to the `Call` event payload. +- A unit test asserts that multiple positional arguments (e.g. `a`, `b`) are present with correct encoded values. +- Varargs/kwargs and positional-only coverage are tracked in separate issues (see ISSUE-002, ISSUE-005). + ### Status -Not started +Done + +Implemented for positional (and pos-or-keyword) arguments on function entry +using `sys._getframe(0)` and `co_varnames[:co_argcount]`, with counting fixed to +use `co_argcount` directly (includes positional-only; avoids double-counting). +Values are encoded via `encode_value` and attached to the `Call` event. Tests +validate correct presence and values. Varargs/kwargs remain covered by +ISSUE-002. # Issues Breaking Declared Relations @@ -29,3 +42,256 @@ Blah blah bleh ## REL-002 ... ``` + +## ISSUE-002 +### Description +Capture all Python argument kinds on function entry: positional-only, +pos-or-kw, keyword-only, plus varargs (`*args`) and kwargs (`**kwargs`). Extend +the current implementation that uses `co_argcount` and `co_varnames` to also +leverage `co_posonlyargcount` and `co_kwonlyargcount`, and detect varargs/kwargs +via code flags. Encode `*args` as a list value and `**kwargs` as a mapping value +to preserve structure. + +### Definition of Done +- All argument kinds are captured on function entry: positional-only, pos-or-keyword, keyword-only, varargs (`*args`), and kwargs (`**kwargs`). +- `*args` is encoded as a list value; `**kwargs` is encoded as a mapping value. +- Positional-only and keyword-only parameters are included using `co_posonlyargcount` and `co_kwonlyargcount`. +- Comprehensive tests cover each argument kind and validate the encoded structure and values. + +### Status +Done + +All argument kinds are captured on function entry, including kwargs with +structured encoding. Varargs are preserved as `Tuple` (per CPython), and +`**kwargs` are encoded as a `Sequence` of 2-element `Tuple`s `(key, value)` +with string keys, enabling lossless downstream analysis. The updated test +`test_all_argument_kinds_recorded_on_py_start` verifies the behavior. + +Note: While the original Definition of Done referenced a mapping value kind, +the implementation follows the proposed approach in ISSUE-008 to represent +kwargs as a sequence of tuples using existing value kinds. + +## ISSUE-005 +### Description +Include positional-only parameters in argument capture. The current logic uses +only `co_argcount` for the positional slice, which excludes positional-only +arguments (PEP 570). As a result, names before the `/` in a signature like +`def f(p, /, q, *args, r, **kwargs)` are dropped. + +### Definition of Done +- Positional-only parameters are included in the captured argument set. +- The selection of positional names accounts for `co_posonlyargcount` in addition to `co_argcount`. +- Tests add a function with positional-only parameters and assert their presence and correct encoding. + +### Status +Done + +Implemented by selecting positional names from `co_varnames` using +`co_argcount` directly (which already includes positional-only per CPython 3.8+). +This prevents double-counting and keeps indexing stable. Tests in +`test_all_argument_kinds_recorded_on_py_start` assert presence of the +positional-only parameter `p` and pass. + +## ISSUE-003 +### Description +Avoid defensive fallback in argument capture. The current change swallows +failures to access the frame/locals and proceeds with empty `args`. Per +`rules/source-code.md` ("Avoid defensive programming"), we should fail fast when +encountering such edge cases. + +### Definition of Done +- Silent fallbacks that return empty arguments on failure are removed. +- The recorder raises a clear, actionable error when it cannot access frame/locals. +- Tests verify the fail-fast path. + +### Status +Done + +`RuntimeTracer::on_py_start` now returns `PyResult<()>` and raises a +`RuntimeError` when frame/locals access fails; `callback_py_start` propagates +the error to Python. A pytest (`tests/test_fail_fast_on_py_start.py`) asserts +the fail-fast behavior by monkeypatching `sys._getframe` to raise. + +## ISSUE-004 +### Description +Stabilize string value encoding for arguments and tighten tests. The new test +accepts either `String` or `Raw` kinds for the `'x'` argument, which can hide +regressions. We should standardize encoding of `str` as `String` (or document +when `Raw` is expected) and update tests to assert the exact kind. + +### Definition of Done +- String values are consistently encoded as `String` (or the expected canonical kind), with any exceptions explicitly documented. +- Tests assert the exact kind for `str` arguments and fail if an unexpected kind (e.g., `Raw`) is produced. +- Documentation clarifies encoding rules for string-like types to avoid ambiguity in future changes. + +### Status +Done + +Stricter tests now assert `str` values are encoded as `String` with the exact text payload, and runtime docs clarify canonical encoding. No runtime logic change was required since `encode_value` already produced `String` for Python `str`. + +## ISSUE-006 +### Description +Accidental check-in of Cargo cache/artifact files under `codetracer-python-recorder/.cargo/**` (e.g., `registry/CACHEDIR.TAG`, `.package-cache`). These are build/cache directories and should be excluded from version control. + +### Definition of Done +- Add ignore rules to exclude Cargo cache directories (e.g., `.cargo/**`, `target/**`) from version control. +- Remove already-checked-in cache files from the repository. +- Verify the working tree is clean after a clean build; no cache artifacts appear as changes. + +### Status +Done + +## ISSUE-007 +### Description +Immediately stop tracing when any monitoring callback raises an error. + +Current behavior: `RuntimeTracer::on_py_start` intentionally fails fast when it +cannot capture function arguments (e.g., when `sys._getframe` is unavailable or +patched to raise). The callback error is propagated to Python via +`callback_py_start` (it returns the `PyResult` from `on_py_start`). However, the +tracer remains installed and active after the error. As a result, any further +Python function start (even from exception-handling or printing the exception) +triggers `on_py_start` again, re-raising the same error and interfering with the +program’s own error handling. + +This is observable in `codetracer-python-recorder/tests/test_fail_fast_on_py_start.py`: +the test simulates `_getframe` failure, which correctly raises in `on_py_start`, +but `print(e)` inside the test’s `except` block invokes codec machinery that +emits additional `PY_START` events. Those callbacks raise again, causing the test +to fail before reaching its assertions. + +### Impact +- Breaks user code paths that attempt to catch and handle exceptions while the + tracer is active — routine operations like `print(e)` can cascade failures. +- Hard to debug because the original error is masked by subsequent callback + errors from unrelated modules (e.g., `codecs`). + +### Proposed Solution +Fail fast and disable tracing at the first callback error. + +Implementation sketch: +- In each callback wrapper (e.g., `callback_py_start`), if the underlying + tracer method returns `Err`, immediately disable further monitoring before + returning the error: + - Set events to `NO_EVENTS` (via `set_events`) to prevent any more callbacks. + - Unregister all previously registered callbacks for our tool id. + - Optionally call `finish()` on the tracer to flush/close writers. + - Option A (hard uninstall): call `uninstall_tracer(py)` to release tool id + and clear the registry. This fully tears down the tracer. Note that the + high-level `ACTIVE` flag in `lib.rs` is not updated by `uninstall_tracer`, + so either: + - expose an internal “deactivate_from_callback()” in `lib.rs` that clears + `ACTIVE`, or + - keep a soft-stop in `tracer.rs` by setting `NO_EVENTS` and unregistering + callbacks without touching `ACTIVE`, allowing `stop_tracing()` to be a + no-op later. + - Ensure reentrancy safety: perform the disable sequence only once (e.g., with + a guard flag) to avoid nested teardown during callback execution. + +Behavioral details: +- The original callback error must still be propagated to Python so the user + sees the true failure cause, but subsequent code should not receive further + monitoring callbacks. +- If error occurs before activation gating triggers, the disable sequence should + still run to avoid repeated failures from unrelated modules importing. + +### Definition of Done +- On any callback error (at minimum `on_py_start`, and future callbacks that may + return `PyResult`), all further monitoring callbacks from this tool are + disabled immediately within the same GIL context. +- The initial error is propagated unchanged to Python. +- The failing test `test_fail_fast_on_py_start.py` passes: after the first + failure, `print(e)` does not trigger additional tracer errors. +- Writers are flushed/closed or left in a consistent state (documented), and no + additional events are recorded after disablement. +- Unit/integration tests cover: error in `on_py_start`, repeated calls after + disablement are no-ops, and explicit `stop_tracing()` is safe after a + callback-induced shutdown. + +### Status +Done + +Implemented soft-stop on first callback error in `callback_py_start`: +on error, the tracer finishes writers, unregisters callbacks for the +configured mask, sets events to `NO_EVENTS`, clears the registry, and +records `global.mask = NO_EVENTS`. The original error is propagated to +Python, and subsequent `PY_START` events are not delivered. This keeps the +module-level `ACTIVE` flag unchanged until `stop_tracing()`, making the +shutdown idempotent. The test `tests/test_fail_fast_on_py_start.py` +exercises the behavior by re-running the program after the initial failure. + +## ISSUE-008 +### Description +Provide structured encoding for kwargs (`**kwargs`) on function entry. The +current backend encodes kwargs as `Raw` text because the `runtime_tracing` +format lacks a mapping value. Introduce a mapping representation so kwargs can +be recorded losslessly with key/value structure and recursively encoded values. + +### Definition of Done +- `runtime_tracing` supports a mapping value kind (e.g., `Map` with string keys). +- `RuntimeTracer::encode_value` encodes Python `dict` to the mapping kind with + recursively encoded values; key type restricted to `str` (non-`str` keys may + be stringified or rejected, behavior documented). +- `on_py_start` records `**kwargs` using the new mapping encoding. +- Tests verify kwargs structure and values; large and nested kwargs are covered. + +### Proposed solution +- We can represent our `Map` as a sequenced of tuples. This way we can use the current value record types to encode dictionaries. +- In the Python recorder, downcast to `dict` and iterate items, encoding values + recursively; keep behavior minimal and fail fast on unexpected key types per + repo rules (no defensive fallbacks). + +### Dependent issues +- Blocks completion of ISSUE-002 + +### Status +Done + +Implemented structured kwargs encoding in the Rust tracer by representing +Python `dict` as a `Sequence` of `(key, value)` `Tuple`s, with keys encoded as +`String` when possible. Tests in +`codetracer-python-recorder/test/test_monitoring_events.py` validate that +kwargs are recorded structurally. This fulfills the goal without introducing a +new mapping value kind, per the proposed solution. + + +## ISSUE-009 +### Description +Unify list/sequence `lang_type` naming across recorders. The Rust tracer now +emits `TypeKind::Seq` with name "List" for Python `list`, while the +pure-Python recorder uses "Array". This divergence can fragment the trace +schema and complicate downstream consumers. + +### Definition of Done +- Both recorders emit the same `lang_type` for Python list values. +- Fixtures and docs/spec are updated to reflect the chosen term. +- Cross-recorder tests pass with consistent types. + +### Proposed solution +- We will use "List" in order to match existing Python nomenclature + +### Status +Low priority. We won't work on this unless it blocks another issue. + + +## ISSUE-010 +### Description +Clarify scope of dict structural encoding and key typing. The current change +encodes any Python `dict` as a `Sequence` of `(key, value)` tuples and falls +back to generic encoding for non-string keys. Repo rules favor fail-fast over +defensive fallbacks, and ISSUE-008 focused specifically on `**kwargs`. + +### Definition of Done +- Decide whether structural dict encoding should apply only to kwargs or to all + dict values; document the choice. +- If limited to kwargs, restrict structured encoding to kwargs capture sites. +- If applied generally, define behavior for non-string keys (e.g., fail fast) + and add tests for nested and non-string-key dicts. + +### Proposed solution +- Prefer failing fast on non-string keys in contexts representing kwargs; if + general dict encoding is retained, update the spec and tests and remove the + defensive fallback for key encoding. + +### Status +Low priority. We won't work on this until a user reports that it causes issues.