fix(stream): hold a bare </>/< followed only by whitespace - #26
Merged
Merged
Conversation
TinySweeper flagged a real streaming regression in the prior commit:
the complete-tag regex now accepts a space before the DSML/namespace
marker (`< | DSML | invoke`), but the streaming scrubber's hold-back
list only matched fixed literal opener prefixes ("<invoke ", "<|DSML",
...). None of those start with a space, so a fragment boundary landing
on "< " (bracket plus whitespace, marker not arrived yet) released it
as plain text — and once split from its bracket, the marker that
followed could never complete an opener, silently dropping the call.
Add `trailing_bare_angle_bracket`: hold a `<` or `</` at the very end
of the buffered tail when it is followed only by whitespace so far.
Self-correcting once more text arrives — it only re-fires while the
tail's end still looks like a bare bracket with nothing conclusive
after it, exactly mirroring how a lone `<` was already held before
this gap.
Test plan:
- New test `a_whitespace_prefixed_dsml_opener_split_before_the_marker_is_held`
reproduces TinySweeper's exact scenario (feed "< ", then the rest) —
fails without the fix, passes with it.
- `cargo test -p tinytools-agent`: 323 passed, including
`plural_tool_calls_prose_is_not_held` (confirms ordinary `<` in
prose is unaffected).
- `cargo fmt --all -- --check`: clean.
- `cargo clippy --all-targets --all-features -- -D warnings`: clean.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Comment |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete 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
Before merge
How this fits togetherflowchart LR
n0["hold_from<br/>changed<br/>1 finding"]:::blocking
n1["dsml_split_across_fragments_is_scrubbed<br/>changed"]:::changed
n2["len"]:::impacted
n3["feed"]:::impacted
n4["scrub_all"]:::impacted
n5["flush"]:::impacted
n6["...d_string_is_released_once_and_never_shown"]:::impacted
n7["options"]:::impacted
n0 -->|calls| n2
n1 -->|calls| n4
n1 -->|tests| n4
n3 -->|calls| n0
n3 -->|calls| n2
n3 -->|calls| n7
n4 -->|calls| n2
n4 -->|calls| n3
n4 -->|calls| n5
n5 -->|calls| n7
n6 -->|calls| n3
n6 -->|tests| n3
n6 -->|calls| n5
n6 -->|tests| 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
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0046 · 82,754 in / 13,732 out · 20,366 cached (25%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 276 embedded
critique: $0.0014 · 33,637 in / 3,429 out · 2,881 cached (9%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0007 · 29,208 in / 686 out · 1,869 cached (6%) · gpt-5.6-luna
tests: $0.0011 · 13,117 in / 4,821 out · 12,800 cached (98%) · deepseek/deepseek-v4-flash
description: $0.0005 · 4,466 in / 1,548 out · 2,304 cached (52%) · deepseek/deepseek-v4-flash
| /// landing on the bracket and its whitespace (`"< "`) is released as plain | ||
| /// text before the marker arrives, and the opener is unrecoverable once | ||
| /// split from its `<`. | ||
| fn trailing_bare_angle_bracket(tail: &str) -> Option<usize> { |
There was a problem hiding this comment.
Hold partially received XML prefixes after the bracket
This only detects a trailing < or </, so it stops protecting the opener as soon as any prefix character arrives. For example, feeding "< | DSML | D" or "< atem:" releases the text because the tail no longer ends in a bare angle bracket, even though both are valid prefixes of < | DSML | invoke ... and < atem:invoke ... respectively. The next fragment ("SML | invoke ..." or "invoke ...") then lacks the original <... prefix and the call is permanently lost. Hold these whitespace-prefixed DSML and namespace prefixes until they are either recognized as a complete opener or disambiguated as ordinary text.
[RULE] partial-input-loss ·
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #25, which already merged before this finding was addressed. TinySweeper correctly flagged a real streaming regression in that PR: the complete-tag regex now accepts a space before the DSML/namespace marker (
< | DSML | invoke), but the streaming scrubber's hold-back list (hold_frominstream/mod.rs) only matched fixed literal opener prefixes ("<invoke ","<|DSML", ...). None of those start with a space, so a fragment boundary landing on"< "(bracket plus whitespace, marker not arrived yet) was released as plain text — and once split from its bracket, the marker that followed could never complete an opener, silently dropping the call.Fix
Added
trailing_bare_angle_bracket: holds a<or</at the very end of the buffered tail when it is followed only by whitespace so far. Self-correcting once more text arrives — it only re-fires while the tail's end still looks like a bare bracket with nothing conclusive after it, mirroring how a lone<was already held before this gap.Test plan
a_whitespace_prefixed_dsml_opener_split_before_the_marker_is_heldreproduces TinySweeper's exact scenario (feed"< ", then the rest) — fails without the fix, passes with it.cargo test -p tinytools-agent: 323 passed, includingplural_tool_calls_prose_is_not_held(confirms ordinary<in prose is unaffected).cargo fmt --all -- --check: clean.cargo clippy --all-targets --all-features -- -D warnings: clean.Ref: #25 (discussion: #25 (comment))