feat(dialect): typed replay messages (to_typed_messages) - #41
Conversation
Introduce a new native dialect module that provides foundational type definitions and a native implementation for the agent's dialect system. This change establishes the core type infrastructure needed to support dialect-specific behavior in the tinytools agent. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a `to_typed_messages` method to the native dialect that unpacks the envelope format into structured fields, and verify it produces the same output as the existing packed format. This enables downstream consumers to work with the message content directly without parsing the envelope string. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 3 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Could not review: crates/tinytools-agent/src/dialect/mod.rs, crates/tinytools-agent/src/dialect/native.rs, crates/tinytools-agent/src/dialect/test.rs, crates/tinytools-agent/src/dialect/types.rs Before merge
How this fits togetherflowchart LR
n0["NativeDialect<br/>changed"]:::changed
n1["DialectMessage<br/>changed<br/>1 finding"]:::flagged
n2["DialectRole<br/>changed<br/>1 finding"]:::flagged
n3["response"]:::impacted
n4["ToolDialect"]:::impacted
n5["TranscriptEntry"]:::impacted
n6["...ect_values_fallback_and_protocol_metadata"]:::impacted
n7["one"]:::impacted
n0 -->|implements| n4
n1 -->|uses| n2
n4 -->|uses| n1
n4 -->|uses| n5
n5 -->|uses| n1
n6 -->|calls| n3
n6 -->|tests| n3
n6 -->|calls| n7
n6 -->|tests| n7
n7 -->|uses| n5
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (2)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe dialect API now supports replaying transcript history as typed messages. ChangesTyped dialect replay
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The change adds structured native replay while retaining existing packed replay behavior. No actionable merge-blocking issue is established; it is mergeable subject to normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new API changes message representation rather than tool permissions. Existing packed replay and call/result pairing checks remain available. Downstream adapter behavior is not fully established, particularly for absent versus empty assistant text. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
A rabbit checks the message trail, Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ce3d9c29f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// The native tool calls an assistant message made. Empty everywhere except | ||
| /// a typed native assistant turn. | ||
| #[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
| pub tool_calls: Vec<NativeToolCall>, |
There was a problem hiding this comment.
Adding fields to the public, non-non_exhaustive DialectMessage is source-incompatible because every downstream struct literal must now initialize three additional fields. The workspace still advertises version 0.5.0, so this breaking public-surface change needs a 0.6.0 version bump or a design that does not extend the existing public struct.
AGENTS.md reference: AGENTS.md:L264-L266
Useful? React with 👍 / 👎.
| #[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
| pub tool_calls: Vec<NativeToolCall>, |
There was a problem hiding this comment.
Add a serialization regression test for DialectMessage covering the three new field names, their emitted values, and their default omission during serialization. The new replay test only examines Rust fields and manually repacks envelopes, so it would not catch an accidental serde rename or removal of a skip_serializing_if rule even though this type is a persisted/provider-facing payload whose wire representation must be pinned.
AGENTS.md reference: AGENTS.md:L175-L177
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinytools-agent/src/dialect/mod.rs, crates/tinytools-agent/src/dialect/native.rs, crates/tinytools-agent/src/dialect/test.rs, crates/tinytools-agent/src/dialect/types.rs.
$0.0034 · 26,096 in / 9,952 out · 9,984 cached (38%) · ladder/vectors, deepseek/deepseek-v4-flash · 529 embedded
tests: $0.0019 · 15,113 in / 4,102 out · 3,072 cached (20%) · deepseek/deepseek-v4-flash
description: $0.0010 · 6,250 in / 3,222 out · 2,304 cached (37%) · deepseek/deepseek-v4-flash
| /// [`ToolDialect::to_typed_messages`](super::ToolDialect::to_typed_messages) | ||
| /// emits the same round with the structure in the typed fields below and | ||
| /// `content` holding plain text only. | ||
| #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] |
There was a problem hiding this comment.
Pin the serde representation of the new DialectMessage fields
DialectMessage is a public payload type that is serialized/deserialized. The diff adds three new fields (tool_calls, tool_call_id, reasoning_content) with #[serde(default, skip_serializing_if = ...)] to maintain backward compatibility, but there is no test that asserts the JSON shape of a DialectMessage with and without these fields remains stable. The repository's coding rules require payload types to pin their serde representation in a unit test. Without such a test, a future change that accidentally alters the wire format (e.g., changes the skip_serializing_if condition or adds a rename) would go undetected.
[RULE] missing-serde-pin-test ·
| } | ||
|
|
||
| #[test] | ||
| fn native_typed_replay_drops_unpaired_cycles_like_the_packed_replay() { |
There was a problem hiding this comment.
Verify that the native dialect's packed path also drops unpaired cycles
The test asserts that to_typed_messages and to_provider_messages return the same result for an unpaired tool call, but it does not independently verify that the packed path (to_provider_messages) actually drops the cycle. If the packed path were changed to keep unpaired cycles (or if it never dropped them in the first place), this test would still pass because both paths return the same (wrong) output. The test should either assert an expected length (e.g., assert_eq!(typed.len(), 1)) or explicitly verify that the assistant turn is absent, to pin the dropping behavior independently of the equivalence assertion.
[RULE] unverified-assumption ·
1 participant
Design note
Current state.
ToolDialect::to_provider_messagesreturnsDialectMessage { role, content, extra_metadata }. For the native dialect a tool round is packed intocontent(the{content, tool_calls}/{tool_call_id, content}envelopes), so every host row bridge has to parse the envelope back out (the follow-up to #40 moved that codec here; hosts still round-trip through it).Target.
DialectMessagegains typed fields (tool_calls,tool_call_id,reasoning_content; serde-default, skipped when empty) andToolDialectgainsto_typed_messages: same pairing/order/metadata asto_provider_messages, but a native tool round keeps its structure in the fields andcontentis plain text. The default implementation returns the packed form, so the XML/P-Format/code dialects are unchanged (they have no structure to unpack).NativeDialectoverrides it.Compatibility.
to_provider_messagesis untouched, so every existing caller and byte of the packed form is identical. New fields default empty; new constructorsDialectMessage::assistant_with_calls/tool_result. A struct literal ofDialectMessageoutside this crate needs the three new fields (useDialectMessage::new).Proof.
native_typed_replay_matches_the_packed_replay(same rows; re-encoding the typed message gives exactly the packed row),native_typed_replay_drops_unpaired_cycles_like_the_packed_replay,text_dialects_keep_the_packed_form_as_their_typed_form.cargo test -p tinytools-agent386 passed; clippy-D warningsclean.Part of the OpenHuman typed in-memory transcript row cut (tinyagents session PR + OpenHuman PR follow).
Summary by CodeRabbit