Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 155 additions & 4 deletions src/openhuman/tinyflows/caps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,64 @@ pub(crate) fn parse_llm_json(text: &str) -> Option<Value> {
matches!(parsed, Value::Object(_) | Value::Array(_)).then_some(parsed)
}

/// Find and parse a fenced JSON block (```json … ``` or ``` … ```) anywhere
/// in `text`, not just when the whole text starts with it. Returns `None` when
/// no fenced block parses to an object or array.
fn extract_fenced_json_block(text: &str) -> Option<Value> {
let text = text.trim();
// Look for the first opening ``` fence
let fence_start = text.find("```")?;
let after_fence = text[fence_start + 3..].trim();
// Skip optional "json" after the opening fence
let content = after_fence.strip_prefix("json").unwrap_or(after_fence).trim();
// Find the *last* closing ``` (preferring the outermost fence, which
// matches how Markdown renderers treat nested fences — the last ``` is
// the one that closes the block the LLM opened).
let close = content.rfind("```")?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Iterate fenced blocks instead of pairing first with last

When the model includes any earlier fenced example/log before the final JSON answer, this pairs the first opening fence with the last closing fence, so the fenced parse fails over a span containing multiple fences. The code then falls through to balanced-brace extraction, which can return an unrelated earlier object such as an example {"example": true} instead of the later ```json answer, silently feeding the wrong value to the flow/output_parser.

Useful? React with 👍 / 👎.

let inner = content[..close].trim();
let parsed = serde_json::from_str::<Value>(inner).ok()?;
matches!(parsed, Value::Object(_) | Value::Array(_)).then_some(parsed)
}
Comment on lines +439 to +453

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 rfind("```") incorrectly merges two independent fenced blocks

When the LLM response contains more than one fenced code block — e.g. a first block containing a wrong attempt and a second block with the corrected JSON — extract_fenced_json_block opens at the first ``` and closes at the last ``` via rfind. Everything between those two positions becomes inner, which includes the prose and the second opening fence, so serde_json::from_str fails on it and the whole function returns None. The balanced-brace scan would then rescue a valid object anyway, but the "fenced block wins" guarantee documented in the test build_agent_result_prefers_fenced_json_over_balanced_brace_extraction breaks when two blocks are present. Using find instead of rfind for the closing fence (i.e. the first ``` after the opening) would correctly delimit the first block.

Comment on lines +439 to +453

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

rfind picks the wrong closing fence when the response has more than one fenced block.

close = content.rfind("```") scans for the last triple-backtick in the entire remainder of text, not the fence that actually closes the block just opened. This works only when the fenced block is the sole/last fence in the text (as in the new build_agent_result_prefers_fenced_json_over_balanced_brace_extraction test). If the response has an earlier unrelated fence (e.g. an example code block) followed by the JSON fence — a common LLM pattern — rfind spans across both fences, the resulting inner slice is not valid JSON, and the block that should extract cleanly falls through to extract_balanced_json/the {text} fallback instead.

🐛 Proposed fix
-    let close = content.rfind("```")?;
+    let close = content.find("```")?;

This trades the (rare) nested-fence-inside-the-block case for correctly handling the (common) multiple-separate-fences case. Consider adding a regression test with two fenced blocks (e.g. a code example followed by the JSON block) to lock this in.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/tinyflows/caps.rs` around lines 439 - 453, Update
extract_fenced_json_block to locate the closing fence with content.find("```")
rather than rfind, so it closes the first fenced block after the opening fence
and handles multiple separate fenced blocks correctly. Preserve the existing
JSON parsing and object/array validation behavior, and add a regression test
covering an unrelated fenced block followed by a JSON fenced block.


/// Find and parse the first balanced `{…}` or `[…}` span in `text`. Walks

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Docstring typo: […} should be […]

The closing delimiter in the doc comment reads […} (a mix of bracket and brace), but the matching pair for [ is ]. The same typo appears in the inline comment on line 920 inside build_agent_result ({…} / […}). Neither affects runtime behavior, but it's misleading for anyone reading the docs.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

/// through the text character by character tracking brace depth; when a span
/// is found and parses as JSON, it is returned. `None` when no span parses.
fn extract_balanced_json(text: &str) -> Option<Value> {
let text = text.trim();
let bytes = text.as_bytes();
let len = bytes.len();
for start in 0..len {
let open_byte = bytes[start];
let (open, close) = match open_byte {
b'{' => (b'{', b'}'),
b'[' => (b'[', b']'),
_ => continue,
};
let mut depth = 0u32;
for end in start..len {
let b = bytes[end];
if b == open {
depth = depth.checked_add(1)?;
} else if b == close {
depth = depth.checked_sub(1)?;
if depth == 0 {
// Found a balanced span — try to parse it.
let candidate = &text[start..=end];
Comment on lines +471 to +478

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect quoted strings while scanning embedded JSON

When the valid embedded JSON contains a structural character inside a string, for example prose followed by {"text":"use } as a delimiter"}, this byte scanner decrements depth for the quoted } and tries to parse an incomplete object, then breaks without reconsidering the real closing brace. Those structured agent outputs still fall back to {text, agent_ref} and can fail the downstream output_parser even though the JSON object was valid.

Useful? React with 👍 / 👎.

if let Ok(parsed) = serde_json::from_str::<Value>(candidate) {
if matches!(parsed, Value::Object(_) | Value::Array(_)) {
return Some(parsed);
}
}
// Span didn't parse; continue scanning from the position
// after this false-positive open byte.
break;
}
}
}
}
None
Comment on lines +458 to +491

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 String-unaware brace scanning drops valid JSON with braces in values

extract_balanced_json scans bytes and increments/decrements depth on every {/} or [/], regardless of whether the byte is inside a quoted string. When a JSON string value itself contains a brace or bracket — e.g. {"message": "Use {name} as a placeholder"} or {"sql": "SELECT {col} FROM t"} — the { inside the string increments depth to 2, so the outer closing } only decrements it to 1 and the balanced span is never found. The function returns None, falling through to the {text, agent_ref} shape and still breaking the downstream output_parser for those inputs. Adding a test case for this pattern would expose the gap — e.g. The result is: {"message": "Use {name}"} should extract the object but currently will not.

}
Comment on lines +458 to +492

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Balanced-span scan is not string-aware and can silently fail to extract valid JSON — plus quadratic worst case.

This is the last recovery strategy before falling back to {text, agent_ref}, so its correctness matters most. Two issues:

  1. Depth counting operates on raw bytes with no notion of JSON string literals. A string value containing an unescaped bracket/brace of the same type being scanned (e.g. {"note": "see [draft]"} when scanning [/], or ["a]b"]) drives depth to 0 prematurely. The resulting candidate fails to parse, the inner loop breaks, and — because the outer loop never revisits start — the correct span starting at that position is never retried. For a plain array like ["a]b", "c"] this makes the function return None even though valid JSON is present.
  2. When no closing bracket is ever found for a given start, the inner loop scans to len before moving on; repeated across many candidate start positions this is O(n²) in the worst case (e.g. text containing many unmatched {/[ characters).
🛡️ Proposed fix (adds string/escape awareness)
 fn extract_balanced_json(text: &str) -> Option<Value> {
     let text = text.trim();
     let bytes = text.as_bytes();
     let len = bytes.len();
     for start in 0..len {
         let open_byte = bytes[start];
         let (open, close) = match open_byte {
             b'{' => (b'{', b'}'),
             b'[' => (b'[', b']'),
             _ => continue,
         };
         let mut depth = 0u32;
+        let mut in_string = false;
+        let mut escaped = false;
         for end in start..len {
             let b = bytes[end];
-            if b == open {
-                depth = depth.checked_add(1)?;
-            } else if b == close {
-                depth = depth.checked_sub(1)?;
-                if depth == 0 {
-                    // Found a balanced span — try to parse it.
-                    let candidate = &text[start..=end];
-                    if let Ok(parsed) = serde_json::from_str::<Value>(candidate) {
-                        if matches!(parsed, Value::Object(_) | Value::Array(_)) {
-                            return Some(parsed);
-                        }
-                    }
-                    // Span didn't parse; continue scanning from the position
-                    // after this false-positive open byte.
-                    break;
-                }
-            }
+            if in_string {
+                if escaped {
+                    escaped = false;
+                } else if b == b'\\' {
+                    escaped = true;
+                } else if b == b'"' {
+                    in_string = false;
+                }
+                continue;
+            }
+            match b {
+                b'"' => in_string = true,
+                _ if b == open => depth = depth.checked_add(1)?,
+                _ if b == close => {
+                    depth = depth.checked_sub(1)?;
+                    if depth == 0 {
+                        let candidate = &text[start..=end];
+                        if let Ok(parsed) = serde_json::from_str::<Value>(candidate) {
+                            if matches!(parsed, Value::Object(_) | Value::Array(_)) {
+                                return Some(parsed);
+                            }
+                        }
+                        break;
+                    }
+                }
+                _ => {}
+            }
         }
     }
     None
 }

For the quadratic-scan concern, consider bounding the scanned length (e.g. only scan up to some max prefix of final_text) as a cheap mitigation if unbounded model output can reach this path. Add regression tests covering a string value with an embedded stray bracket/brace to lock in the fix.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn extract_balanced_json(text: &str) -> Option<Value> {
let text = text.trim();
let bytes = text.as_bytes();
let len = bytes.len();
for start in 0..len {
let open_byte = bytes[start];
let (open, close) = match open_byte {
b'{' => (b'{', b'}'),
b'[' => (b'[', b']'),
_ => continue,
};
let mut depth = 0u32;
for end in start..len {
let b = bytes[end];
if b == open {
depth = depth.checked_add(1)?;
} else if b == close {
depth = depth.checked_sub(1)?;
if depth == 0 {
// Found a balanced span — try to parse it.
let candidate = &text[start..=end];
if let Ok(parsed) = serde_json::from_str::<Value>(candidate) {
if matches!(parsed, Value::Object(_) | Value::Array(_)) {
return Some(parsed);
}
}
// Span didn't parse; continue scanning from the position
// after this false-positive open byte.
break;
}
}
}
}
None
}
fn extract_balanced_json(text: &str) -> Option<Value> {
let text = text.trim();
let bytes = text.as_bytes();
let len = bytes.len();
for start in 0..len {
let open_byte = bytes[start];
let (open, close) = match open_byte {
b'{' => (b'{', b'}'),
b'[' => (b'[', b']'),
_ => continue,
};
let mut depth = 0u32;
let mut in_string = false;
let mut escaped = false;
for end in start..len {
let b = bytes[end];
if in_string {
if escaped {
escaped = false;
} else if b == b'\\' {
escaped = true;
} else if b == b'"' {
in_string = false;
}
continue;
}
match b {
b'"' => in_string = true,
_ if b == open => depth = depth.checked_add(1)?,
_ if b == close => {
depth = depth.checked_sub(1)?;
if depth == 0 {
let candidate = &text[start..=end];
if let Ok(parsed) = serde_json::from_str::<Value>(candidate) {
if matches!(parsed, Value::Object(_) | Value::Array(_)) {
return Some(parsed);
}
}
break;
}
}
_ => {}
}
}
}
None
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/tinyflows/caps.rs` around lines 458 - 492, Update
extract_balanced_json to track JSON string-literal and escape state while
counting delimiters, so brackets or braces inside quoted values do not affect
balancing and valid spans are still parsed. Also bound or otherwise avoid
rescanning the full remaining text for every unmatched opening delimiter to
prevent quadratic behavior, and add regression coverage for embedded stray
delimiters in string values.


/// Select the model an `agent` node completion actually runs on.
///
/// `resolved_model` is what [`create_chat_provider`] returned for the node's
Expand Down Expand Up @@ -841,20 +899,40 @@ pub(crate) fn build_harness_run_prompt(request: &Value) -> String {
/// still applies.
pub(crate) fn build_agent_result(agent_ref: &str, final_text: &str, request: &Value) -> Value {
if structured_output_requested(request) {
// Try 1: bare JSON parsing and full-text fence blocks (existing).
if let Some(parsed) = parse_llm_json(final_text) {
tracing::debug!(
target: "flows",
agent_ref,
"[flows] agent_runner: structured output parsed from harness turn"
"[flows] agent_runner: structured output parsed from harness turn (parse_llm_json)"
);
return parsed;
}
// Try 2: find a fenced ```json … ``` block anywhere in the text.
if let Some(parsed) = extract_fenced_json_block(final_text) {
tracing::debug!(
target: "flows",
agent_ref,
"[flows] agent_runner: structured output extracted from fenced JSON block in prose"
);
return parsed;
}
// Try 3: find the first balanced {…} / […} span in the text (handles
// cases where the LLM wraps JSON in prose without fence blocks).
if let Some(parsed) = extract_balanced_json(final_text) {
tracing::debug!(
target: "flows",
agent_ref,
"[flows] agent_runner: structured output extracted from balanced brace/bracket span in prose"
);
return parsed;
}
tracing::warn!(
target: "flows",
agent_ref,
"[flows] agent_runner: structured output requested but the harness turn did not parse \
as JSON — falling back to the {{text}} shape (the output_parser sub-port may still \
coerce it)"
"[flows] agent_runner: structured output requested but none of the extraction strategies \
produced valid JSON — falling back to the {{text}} shape (the output_parser sub-port may \
still coerce it)"
);
}
json!({ "text": final_text, "agent_ref": agent_ref })
Expand Down Expand Up @@ -5552,4 +5630,77 @@ mod tests {
assert_eq!(value["usage"]["charged_amount_usd"], 0.125);
assert_eq!(value["reasoning_content"], "private chain");
}

// ── build_agent_result improvements (issue #5151) ────────────────────

#[test]
fn build_agent_result_extracts_embedded_json_from_prose_text() {
// When the agent's final text wraps JSON in prose without fence
// blocks (e.g. the LLM explains the result before outputting the
// data), build_agent_result must still extract the object rather than
// falling back to {text, agent_ref} which kills the downstream
// output_parser.
let request = json!({
"output_parser": {
"schema": { "type": "object", "required": ["name"] }
}
});
let result = build_agent_result(
"agent-1",
"The result is: { \"name\": \"Alice\", \"age\": 30 }",
&request,
);
assert_eq!(result, json!({ "name": "Alice", "age": 30 }));
}

#[test]
fn build_agent_result_extracts_embedded_array_from_prose_text() {
let request = json!({
"output_parser": {
"schema": { "type": "array" }
}
});
let result = build_agent_result(
"agent-1",
"Here is the list: [1, 2, 3]",
&request,
);
assert_eq!(result, json!([1, 2, 3]));
}

#[test]
fn build_agent_result_falls_back_to_text_when_no_json_found_in_prose() {
// Pure prose with no JSON-like content must still fall back to the
// safe {text, agent_ref} shape.
let request = json!({
"output_parser": {
"schema": { "type": "object", "required": ["name"] }
}
});
let result = build_agent_result(
"agent-1",
"I searched for the information but could not find it.",
&request,
);
assert_eq!(
result,
json!({ "text": "I searched for the information but could not find it.",
"agent_ref": "agent-1" })
);
}

#[test]
fn build_agent_result_prefers_fenced_json_over_balanced_brace_extraction() {
// When both a fenced block and loose prose-with-JSON are present,
// the fenced block wins (it's the canonical / better-specified
// format).
let request = json!({
"output_parser": {
"schema": { "type": "object" }
}
});
let text = "Some text\n```json\n{\"from_fence\": true}\n```\nmore text { \"from_brace\": true }";
let result = build_agent_result("agent-1", text, &request);
assert_eq!(result, json!({ "from_fence": true }));
}
Comment on lines +5620 to +5692

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

These additions keep growing an already oversized module.

caps.rs already spans well past 5,600 lines; this PR adds two new extraction helpers, a reworked pipeline branch, and ~70 lines of tests on top of that. As per coding guidelines, "Prefer files of approximately 500 lines or fewer and keep modules small, single-responsibility, and composed through clear boundaries."

Consider extracting parse_llm_json, extract_fenced_json_block, extract_balanced_json, and their dedicated tests into a focused sibling module (e.g. src/openhuman/tinyflows/json_extract.rs), re-exported for use by build_agent_result. This is a larger, file-wide effort beyond just this diff, but pays off in reviewability/testability for a module that will likely keep growing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/tinyflows/caps.rs` around lines 5620 - 5692, Extract
parse_llm_json, extract_fenced_json_block, and extract_balanced_json from
caps.rs into a focused sibling module such as json_extract.rs, moving their
dedicated tests there as well. Expose the required functions to
build_agent_result through the module boundary, preserving fenced JSON
precedence, balanced extraction, and existing fallback behavior.

Source: Coding guidelines

}
Loading