fix: improve root cause detector parsing returned json

[ybdarrenwang]

Description

The root cause analysis pipeline runs in two phases: a) Failure detection — a plain text-mode LLM call whose response we parse into FailureDetectionStructuredOutput. b) Root cause analysis — a Strands structured-output call (structured_output_model=RCAStructuredOutput).

Leaving (a) as a text-mode call was intentional. Per the _call_model docstring, putting the failure taxonomy in the system role makes the model treat it with higher authority and over-apply categories (more false positives), and text mode also avoids tool-use structured-output overhead that was found to degrade detection quality. So (a) keeps a free-text response and parses the JSON out of it.

However, we found that the model often wraps that JSON in prose or markdown, and the parser couldn't recover it:

Failed to parse LLM response for failure detection: 1 validation error for FailureDetectionStructuredOutput
  Invalid JSON: expected value at line 1 column 1 [type=json_invalid, input_value='Looking at this session,...["high"]\n    }\n  ]\n}', input_type=str]

On this failure the detector logs a warning and returns [], so RCA has nothing to analyze and the diagnosis silently comes back empty.

This PR adds tolerant extraction helpers that handle the three shapes the model emits in practice, by scanning for the first balanced {...}/[...] span that actually parses (skipping prose brackets like a mentioned regex). This keeps the intentional text-mode design while making detection resilient to wrapped output.

Related Issues

N/A

Documentation PR

N/A

Type of Change

Bug fix

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

• I ran hatch run prepare

Checklist

• I have read the CONTRIBUTING document
• I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
• My change is focused and reasonably small; I have split unrelated work into separate PRs
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Assessment: Request Changes

The tolerant-extraction approach is sound and correctly fixes the reported prose-wrapped-JSON case (verified locally), and the docstrings clearly explain the intent. Two items should be addressed before merge.

Review Categories

• Correctness: The "first balanced span that parses" heuristic can return a decoy JSON object embedded in the model's prose, which then fails schema validation and reintroduces the silent-empty diagnosis this PR fixes. Consider biasing selection toward the known schema (or largest span).
• Testing: New parsing logic ships without unit tests; the closest existing test only covers the fenced path with a substring assertion. The exact reported scenario (bare JSON after prose) is currently unverified.
• Consistency (minor): _iter_balanced_spans lacks a return type annotation while the rest of the module is fully typed.

Nice job keeping the intentional text-mode design intact while making detection resilient — the reasoning in the docstrings is genuinely helpful.

[poshinchen]

@strandly-the-agent review the changes, it seems like it's the strands-agents structureOutput issue.

[github-actions]

@poshinchen on whether this is a strands-agents structured_output issue — I dug into it:

BedrockModel.structured_output (and the other providers) works by forced tool use (tool_choice={"any": {}} with a Pydantic-derived tool spec) and returns the schema-conformant toolUse.input, never free text. So switching failure detection to structured output would side-step prose-wrapped JSON entirely — your instinct is reasonable, and it's why the extraction workaround exists in the first place.

However, I don't think this is a strands bug — structured output is behaving as designed. The author deliberately kept phase (a) detection in text mode (see the _call_model docstring and PR description): forced-tool / system-role taxonomy was found to inflate false positives and degrade detection quality, whereas phase (b) RCA already uses structured_output_model=RCAStructuredOutput where quality permits. So this PR is the resilience fix for a chosen text-mode design, not a substitute for fixing structured output.

Two takeaways:

• If the goal is to eliminate the parser rather than harden it, the lever is switching detection to structured_output — but that reopens the documented quality tradeoff, so it shouldn't be done casually.
• The quality findings that justify text mode aren't quantified/linked anywhere in-repo. Capturing them (a benchmark link or a short note next to _call_model) would let a future reader retire this extractor if/when the tradeoff changes.

Net: I'd keep the text-mode approach and this hardening as-is.

[github-actions]

Assessment: Approve

The follow-up commit addresses all three points from the previous review:

Resolved

• Correctness: Selection now prefers the balanced span carrying the top-level "errors" key (falling back to first-parseable), so a decoy JSON object in the prose no longer shadows the real payload. Verified.
• Testing: Added targeted tests for the reported scenario — bare object after prose, schema-preferred-over-decoy (with a full _parse_text_result assertion), regex-in-prose skipping, and first-parseable fallback.
• Consistency: _iter_balanced_spans is now annotated -> Iterator[str].

On the structured-output question raised in the thread: this is a deliberate text-mode design tradeoff, not a strands bug — details in my separate comment. Recommend only that the quality rationale be captured in-repo so the extractor can be retired if that tradeoff ever changes. Nice, well-documented iteration.

[strandly-the-agent]

Changes requested — one regression, two-line fix. The extraction fixes the reported shape (verified: prose + pretty-printed JSON goes [] → 1 item vs main). But the new "errors"-key preference returns the first match, so a model that restates one of the prompt's own EXAMPLES blocks before answering now yields a fabricated failure at 0.9 confidence with no warning — main returned [] + a warning. Inline suggestion below; 107 tests still pass with it.

On the structured_output question: agree with the earlier answer — not a strands bug. In strands 1.56.0 structured_output_model is forced tool use (event_loop.py:369-378 sets tool_choice on end_turn) and the result is toolUse.input, so it can't produce prose-wrapped text. This path (_call_model, failure_detector.py:151-176) is raw model.stream() by design and never enters that machinery — the prose wrapping is plain text-mode model behaviour.

✅ Verified (head 692ee13)

• pytest tests/strands_evals/detectors -q → 107 passed; ruff check clean; mypy clean on failure_detector.py.
• ruff format --check (0.16.8) flags test_failure_detector.py:120-123. CI's Lint job runs hatch fmt --linter --check only, so it stays green — but hatch run prepare would have rewritten it; suggestion inline.
• Repros on pr-401 vs main: prose+pretty JSON 0→1, prose+JSON+trailer 0→1, fenced/clean unchanged, no-JSON-at-all still [] + warning. Echoed-example case: main [], pr-401 [('span_id_where_repetition_occurs', [0.9])], pr-401+fix [('real-span', [0.5])].
• Held under attack: braces/quotes inside JSON strings, stray " in prose before the payload, unterminated fence (falls through correctly), wrapper objects {"result": {"errors": …}}, 180 KB of prose → 0.05 s, 3000 unbalanced { → 0.65 s.
• Adversarial pass timed out before reporting; the above is from the reviewer pass plus my own runs, so coverage of exotic inputs is partial.

Questions (non-blocking)

• Is the [...] scan at failure_detector.py:270 ever useful? _iter_balanced_spans already yields nested objects inside an array, and model_validate_json rejects a top-level list anyway — _extract_json('The failures are: [{"location": "s1"}]') returns the inner object, never the array. Dropping it removes a loop level and makes test_extract_json_skips_regex_in_prose vacuous ([a-z0-9-] is never a candidate).
• Title says "root cause detector" but the only caller of _extract_json is failure_detector.py:291; RCA uses structured_output_model and never parses text. fix(detectors): recover prose-wrapped JSON in failure detection would match the change.

Appendix — non-blocking (4)

• ⚪ Docstrings on _extract_json / _extract_balanced_json are ~20 lines each on private helpers; median private-fn docstring in detectors/ is 4 lines. Most of it narrates the code — the one non-obvious thing (why a decoy-resistant discriminator) fits in 2–3 lines.
• ⚪ test_extract_json_prefers_schema_over_decoy (:107) says "the span that validates against it wins" but the code only checks key presence — that's exactly the gap above. Rename to _prefers_errors_key_over_decoy.
• ⚪ test_extract_json_bare_object_after_prose (:106) is compact single-line JSON; the reported shape is pretty-printed. json.dumps(..., indent=2) + assert through _parse_text_result makes the test be the bug report. test_extract_json_falls_back_to_first_parseable (:139) pins an internal choice with no observable difference ({"foo": 1} and raw text both end in [] + warning).
• ⚪ Pre-existing: the fence regex at :200 takes the first fenced block, so a fenced echoed example followed by the fenced real answer returns the echo ({"errors": []} validates → silent []). Unchanged by this PR; noting only because the same "last wins" idea would cover it if you touch that line.

[strandly-the-agent]

First "errors"-bearing span wins, so an echoed prompt example shadows the real answer — and if the echo validates, it's reported as a real failure.

The prompt contains three {"errors": [...]} blocks (failure_detection_v0.py:199, :223+). A response that restates one unfenced before answering hits return candidate on the echo. Verified with the first EXAMPLES block verbatim + a real payload: main → [] + warning; this branch → [('span_id_where_repetition_occurs', [0.9])], no warning, and RCA then analyses a span that doesn't exist. Full-schema validation wouldn't help — a well-formed example validates.

Suggestion: keep the last errors-bearing span (a preamble is the documented failure mode; a trailing recap isn't). 107 tests pass with this.

Suggested change

	if isinstance(parsed, dict) and "errors" in parsed:
	return candidate
	return first_parseable
	if isinstance(parsed, dict) and "errors" in parsed:
	payload = candidate
	return payload or first_parseable

(plus payload: str | None = None next to first_parseable on line 269.)

[strandly-the-agent]

ruff format --check (0.16.8) rewrites this — fits in 120 cols. Won't fail CI (Lint runs the linter only), but hatch run prepare would have applied it.

Suggested change

	real_payload = (
	'{"errors": [{"location": "s1", "category": ["err"], '
	'"confidence": ["high"], "evidence": ["ev"]}]}'
	)
	real_payload = '{"errors": [{"location": "s1", "category": ["err"], "confidence": ["high"], "evidence": ["ev"]}]}'