Ignore fenced examples in GLM fallback - #45
Conversation
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
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 configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughWhen the initial scan finds no calls, GLM fallback parsing now processes kept text ranges separately and skips protected code fences. If it finds GLM calls, it returns the cleaned narrative and calls; otherwise, it retains the original scan result. Tests cover fenced examples and real calls. ChangesGLM Fallback Parsing
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to Fenced examples remain text while real GLM calls are extracted. No merge-blocking issue was established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
A rabbit checks each text-filled line, 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. |
Tiny Sweeper reviewThis pull request prevents fenced code examples from being misclassified as tool-call markup during the GLM fallback path. It introduces a new helper function and adds a related test case. State: Incomplete Review snapshot
Completeness: Incomplete What changedIn `crates/tinytools-agent/src/parse/mod.rs`, the fallback logic that joins narrative parts before GLM parsing is replaced with a call to the new `unprotected_narrative` function (which excludes fenced ranges). A new `unprotected_narrative` function iterates over the `kept` ranges and the protected fenced ranges, extracting only non-fenced text portions, and joins them with newlines. In `crates/tinytools-agent/src/parse/test/markup.rs`, a test line is added to verify that a plain-text fenced block (```text\nshell/command>ls\n```) is not detected as call markup. Features
Tests
FindingsPreviously reported and still active
Resolved this pass
Could not review: crates/tinytools-agent/src/parse/mod.rs, crates/tinytools-agent/src/parse/test/glm.rs, crates/tinytools-agent/src/parse/test/markup.rs Before merge
How this fits togetherflowchart LR
n0["parse_text<br/>changed"]:::changed
n1["parse_response"]:::impacted
n2["parse_response"]:::impacted
n3["parse_tool_calls"]:::impacted
n4["resolve_names"]:::impacted
n5["scan"]:::impacted
n6["ParsedToolCall"]:::impacted
n0 -->|calls| n5
n1 -->|calls| n0
n2 -->|calls| n0
n3 -->|calls| n0
n3 -->|uses| n6
n4 -->|uses| n6
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
|
![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: 8e5008c375
ℹ️ 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".
![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/parse/mod.rs, crates/tinytools-agent/src/parse/test/markup.rs.
$0.0004 · 10,326 in / 1,394 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tests: $0.0002 · 4,308 in / 620 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0001 · 3,789 in / 74 out · 0 cached (0%) · deepseek/deepseek-v4-flash
Adds a test covering a real glm call where a fenced code block contains a tool-call-like line that should not be parsed as a call. The test verifies that only the actual call outside the fence is extracted and that the fenced content remains intact in the text output. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The unprotected narrative function now parses GLM calls directly while preserving fenced examples, instead of returning raw text for a separate parsing pass. This consolidates the GLM fallback logic into a single function that returns both the cleaned narrative and any parsed tool calls, eliminating the need for a second parse step and reducing redundant processing. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test covering the case where a fenced text block falls entirely within a kept range during GLM fallback parsing, ensuring the fence content is preserved while the discarded prefix and the real command after the fence are handled correctly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test now passes the kept range as a slice reference rather than a direct range, matching the updated signature of `parse_unprotected_glm` which expects a slice of ranges. This ensures the test exercises the function with the correct argument type while preserving the original assertion of fenced text retention. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
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/parse/mod.rs, crates/tinytools-agent/src/parse/test/glm.rs, crates/tinytools-agent/src/parse/test/markup.rs.
$0.0008 · 21,425 in / 18,325 out · 9,786 cached (46%) · deepseek/deepseek-v4-flash
tests: $0.0002 · 5,419 in / 155 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0001 · 9,897 in / 16,071 out · 4,411 cached (45%) · deepseek/deepseek-v4-flash
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
Exclude protected fenced examples from the GLM fallback used by
parse_text, so quoted GLM-style commands are not reported as tool-call markup. Add a regression test for a fencedshell/command>lsexample.Verification
cargo fmt --checkcargo test -p tinytools-agent parse::test::markup::prose_a_bare_json_answer_and_a_quoted_example_are_not_markupSummary by CodeRabbit