Skip to content

Commit 24daf32

Browse files
committed
ISSUE-002: Structured encoding for varargs in Rust tracer
- Encode Python tuples as `Tuple` and lists as `Sequence` in `RuntimeTracer::encode_value`, recursively encoding elements. - Leaves kwargs (`dict`) as `Raw` for now; the `runtime_tracing` format has no dedicated mapping variant. - This advances ISSUE-002 by providing structured capture for `*args` instead of raw string fallback, aligning with the tests that accept `Sequence`/`Tuple` for varargs. Notes: - Kept string canonicalization (`String`) intact and existing primitive encodings unchanged. - Did not tighten Python-side tests yet to require structured kwargs, since the underlying format lacks a mapping value; kwargs remain backend-dependent. review(codex): Inline review for ISSUE-002 varargs encoding; update issues.md
1 parent 189271c commit 24daf32

File tree

2 files changed

+65
-6
lines changed

2 files changed

+65
-6
lines changed

codetracer-python-recorder/src/runtime_tracer.rs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::path::{Path, PathBuf};
22

33
use pyo3::prelude::*;
4-
use pyo3::types::{PyAny};
4+
use pyo3::types::{PyAny, PyList, PyTuple};
55

66
use runtime_tracing::{Line, TraceEventsFileFormat, TraceWriter, TypeKind, ValueRecord, NONE_VALUE};
77
use runtime_tracing::NonStreamingTraceWriter;
@@ -84,6 +84,9 @@ impl RuntimeTracer {
8484
/// - `bool` -> `Bool`
8585
/// - `int` -> `Int`
8686
/// - `str` -> `String` (canonical for text; do not fall back to Raw)
87+
/// - common containers:
88+
/// - Python `tuple` -> `Tuple` with encoded elements
89+
/// - Python `list` -> `Sequence` with encoded elements (not a slice)
8790
/// - any other type -> textual `Raw` via `__str__` best-effort
8891
fn encode_value<'py>(&mut self, _py: Python<'py>, v: &Bound<'py, PyAny>) -> ValueRecord {
8992
// None
@@ -107,6 +110,27 @@ impl RuntimeTracer {
107110
return ValueRecord::String { text: s, type_id: ty };
108111
}
109112

113+
// Python tuple -> ValueRecord::Tuple with recursively-encoded elements
114+
if let Ok(t) = v.downcast::<PyTuple>() {
115+
let mut elements: Vec<ValueRecord> = Vec::with_capacity(t.len());
116+
for item in t.iter() {
117+
// item: Bound<PyAny>
118+
elements.push(self.encode_value(_py, &item));
119+
}
120+
let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Tuple, "Tuple");
121+
return ValueRecord::Tuple { elements, type_id: ty };
122+
}
123+
124+
// Python list -> ValueRecord::Sequence with recursively-encoded elements
125+
if let Ok(l) = v.downcast::<PyList>() {
126+
let mut elements: Vec<ValueRecord> = Vec::with_capacity(l.len());
127+
for item in l.iter() {
128+
elements.push(self.encode_value(_py, &item));
129+
}
130+
let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Seq, "Array");
131+
return ValueRecord::Sequence { elements, is_slice: false, type_id: ty };
132+
}
133+
110134
// Fallback to Raw string representation
111135
let ty = TraceWriter::ensure_type_id(&mut self.writer, TypeKind::Raw, "Object");
112136
match v.str() {

issues.md

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,19 @@ to preserve structure.
6262
### Status
6363
Partially done
6464

65-
Implemented varargs (`*args`), keyword-only, and kwargs (`**kwargs`) capture.
66-
Positional-only parameters are now included in the positional slice via
67-
`co_posonlyargcount + co_argcount` (see ISSUE-005). Remaining gap: structured
68-
encoding for `*args`/`**kwargs` per Definition of Done (currently accepted as
69-
backend-dependent; tests allow `Raw`).
65+
Structured encoding for varargs is implemented in the Rust tracer:
66+
- Python `tuple` encodes as `Tuple` (elements recursively encoded).
67+
- Python `list` encodes as `Sequence` (non-slice; elements recursively encoded).
68+
69+
Keyword-only and kwargs capture remain in place; kwargs still encode as `Raw`
70+
due to the lack of a mapping value in the `runtime_tracing` format. See
71+
ISSUE-008 for tracking structured kwargs encoding support.
72+
73+
Note: The Definition of Done mentions "list" for `*args`, but CPython passes
74+
varargs as a tuple; the backend now preserves that shape (`Tuple`), which the
75+
tests also accept. The remaining gap is structured kwargs.
76+
77+
Dependent issues: ISSUE-008
7078

7179
## ISSUE-005
7280
### Description
@@ -214,3 +222,30 @@ Python, and subsequent `PY_START` events are not delivered. This keeps the
214222
module-level `ACTIVE` flag unchanged until `stop_tracing()`, making the
215223
shutdown idempotent. The test `tests/test_fail_fast_on_py_start.py`
216224
exercises the behavior by re-running the program after the initial failure.
225+
226+
## ISSUE-008
227+
### Description
228+
Provide structured encoding for kwargs (`**kwargs`) on function entry. The
229+
current backend encodes kwargs as `Raw` text because the `runtime_tracing`
230+
format lacks a mapping value. Introduce a mapping representation so kwargs can
231+
be recorded losslessly with key/value structure and recursively encoded values.
232+
233+
### Definition of Done
234+
- `runtime_tracing` supports a mapping value kind (e.g., `Map` with string keys).
235+
- `RuntimeTracer::encode_value` encodes Python `dict` to the mapping kind with
236+
recursively encoded values; key type restricted to `str` (non-`str` keys may
237+
be stringified or rejected, behavior documented).
238+
- `on_py_start` records `**kwargs` using the new mapping encoding.
239+
- Tests verify kwargs structure and values; large and nested kwargs are covered.
240+
241+
### Proposed solution
242+
- We can represent our `Map` as a sequenced of tuples. This way we can use the current value record types to encode dictionaries.
243+
- In the Python recorder, downcast to `dict` and iterate items, encoding values
244+
recursively; keep behavior minimal and fail fast on unexpected key types per
245+
repo rules (no defensive fallbacks).
246+
247+
### Dependent issues
248+
- Blocks completion of ISSUE-002
249+
250+
### Status
251+
Not started

0 commit comments

Comments
 (0)