From 9d830e0ee2567ea53d9408dcdebf899e547b4e98 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Apr 2026 05:31:59 +0000 Subject: [PATCH 1/2] Fix correctness bugs: dict mutation, budget boundary, JSON parsing, backend safety P0 fixes: - Fix dict mutation in consensus.py, voting.py, conflicts.py: copy backend_kwargs instead of aliasing, matching synthesize.py pattern - Fix budget off-by-one: record() now uses >= to match exhausted property, consistently enforcing the boundary - Fix JSON extraction: _try_extract_object now tracks string literals so braces inside JSON string values don't break brace matching P1 fixes: - Ollama backend: catch ValueError from response.json() and raise a clear RuntimeError instead of opaque JSONDecodeError - OpenAI backend: explicitly pass stream=False, fix operator precedence in response handling to guard against None message Tests: - Add budget exact-boundary test - Add backend_kwargs mutation tests for consensus and voting - Add JSON parsing tests for braces inside string values https://claude.ai/code/session_01H1AP8Drr8SE7QFevAookwL --- src/broadside_ai/backends/ollama.py | 8 ++++++- src/broadside_ai/backends/openai.py | 5 ++++- src/broadside_ai/budget.py | 2 +- src/broadside_ai/conflicts.py | 2 +- src/broadside_ai/parsing.py | 27 ++++++++++++++++++------ src/broadside_ai/strategies/consensus.py | 2 +- src/broadside_ai/strategies/voting.py | 2 +- tests/test_budget.py | 8 +++++++ tests/test_gather.py | 18 +++++++++++++++- tests/test_synthesize.py | 14 ++++++++++++ 10 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/broadside_ai/backends/ollama.py b/src/broadside_ai/backends/ollama.py index 6431d1c..4ff0ccf 100644 --- a/src/broadside_ai/backends/ollama.py +++ b/src/broadside_ai/backends/ollama.py @@ -42,7 +42,13 @@ async def complete(self, prompt: str, **kwargs: Any) -> AgentResult: async with httpx.AsyncClient(timeout=self.timeout) as client: response = await client.post(f"{self.base_url}/api/generate", json=payload) response.raise_for_status() - data = response.json() + try: + data = response.json() + except ValueError: + raise RuntimeError( + f"Invalid response from Ollama at {self.base_url}. " + "The server may be overloaded or returning an error page." + ) from None except httpx.ConnectError: raise ConnectionError( f"Can't connect to Ollama at {self.base_url}.\n\n" diff --git a/src/broadside_ai/backends/openai.py b/src/broadside_ai/backends/openai.py index 30c52b9..78d23fb 100644 --- a/src/broadside_ai/backends/openai.py +++ b/src/broadside_ai/backends/openai.py @@ -54,12 +54,15 @@ async def complete(self, prompt: str, **kwargs: Any) -> AgentResult: model=kwargs.pop("model", self.model), max_tokens=kwargs.pop("max_tokens", self.max_tokens), messages=[{"role": "user", "content": prompt}], + stream=False, **kwargs, ) latency = (time.perf_counter() - t0) * 1000 choice = response.choices[0] if response.choices else None - text = choice.message.content or "" if choice else "" + text = "" + if choice and choice.message: + text = choice.message.content or "" usage = response.usage tokens_in = usage.prompt_tokens if usage else 0 tokens_out = usage.completion_tokens if usage else 0 diff --git a/src/broadside_ai/budget.py b/src/broadside_ai/budget.py index d2190ff..3542285 100644 --- a/src/broadside_ai/budget.py +++ b/src/broadside_ai/budget.py @@ -36,7 +36,7 @@ def record(self, tokens: int) -> None: """Record tokens consumed by a branch. Raises if budget exceeded.""" with self._lock: self._used += tokens - if self.max_tokens is not None and self._used > self.max_tokens: + if self.max_tokens is not None and self._used >= self.max_tokens: raise BudgetExceeded(self.max_tokens, self._used) @property diff --git a/src/broadside_ai/conflicts.py b/src/broadside_ai/conflicts.py index 57a648e..909f921 100644 --- a/src/broadside_ai/conflicts.py +++ b/src/broadside_ai/conflicts.py @@ -50,7 +50,7 @@ async def detect_conflicts( This is a separate step from synthesis - you can run it independently to audit outputs before deciding how to synthesize. """ - bk = backend_kwargs or {} + bk = dict(backend_kwargs or {}) if model: bk["model"] = model llm = get_backend(backend, **bk) diff --git a/src/broadside_ai/parsing.py b/src/broadside_ai/parsing.py index 20283a2..014210a 100644 --- a/src/broadside_ai/parsing.py +++ b/src/broadside_ai/parsing.py @@ -48,11 +48,24 @@ def _try_extract_object(text: str) -> dict[str, Any] | None: return None depth = 0 - for index in range(start, len(text)): - if text[index] == "{": - depth += 1 - elif text[index] == "}": - depth -= 1 - if depth == 0: - return _try_loads(text[start : index + 1]) + in_string = False + i = start + while i < len(text): + ch = text[i] + if in_string: + if ch == "\\": + i += 2 + continue + if ch == '"': + in_string = False + else: + if ch == '"': + in_string = True + elif ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return _try_loads(text[start : i + 1]) + i += 1 return None diff --git a/src/broadside_ai/strategies/consensus.py b/src/broadside_ai/strategies/consensus.py index 0b3e415..f2c1e9b 100644 --- a/src/broadside_ai/strategies/consensus.py +++ b/src/broadside_ai/strategies/consensus.py @@ -32,7 +32,7 @@ async def synthesize_consensus( 2. Where agents disagreed and what each side said 3. Any facts that only one agent mentioned """ - bk = backend_kwargs or {} + bk = dict(backend_kwargs or {}) if model: bk["model"] = model llm = get_backend(backend, **bk) diff --git a/src/broadside_ai/strategies/voting.py b/src/broadside_ai/strategies/voting.py index 28ae1ab..cd85eab 100644 --- a/src/broadside_ai/strategies/voting.py +++ b/src/broadside_ai/strategies/voting.py @@ -35,7 +35,7 @@ async def synthesize_voting( 2. extract_labels=False: Treat each full output as a "vote" and use the LLM to identify the majority position. """ - bk = backend_kwargs or {} + bk = dict(backend_kwargs or {}) if model: bk["model"] = model llm = get_backend(backend, **bk) diff --git a/tests/test_budget.py b/tests/test_budget.py index f3066e6..e9e292f 100644 --- a/tests/test_budget.py +++ b/tests/test_budget.py @@ -38,3 +38,11 @@ def test_budget_exhausted_flag(): # Manually push past limit to test exhausted check b._used = 1000 assert b.exhausted + + +def test_budget_exact_boundary_raises(): + """record() and exhausted must agree at the exact boundary.""" + b = ScatterBudget(max_tokens=1000) + with pytest.raises(BudgetExceeded): + b.record(1000) + assert b.exhausted diff --git a/tests/test_gather.py b/tests/test_gather.py index 634ba54..d52510c 100644 --- a/tests/test_gather.py +++ b/tests/test_gather.py @@ -1,6 +1,7 @@ -"""Tests for gather result normalization.""" +"""Tests for gather result normalization and JSON parsing.""" from broadside_ai.gather import gather +from broadside_ai.parsing import try_parse_json from tests.conftest import make_result @@ -35,3 +36,18 @@ def test_gather_summary(): assert summary["n_requested"] == 1 assert summary["completed"] == 1 assert summary["n_parsed"] == 0 + + +def test_parse_json_with_braces_in_string_values(): + text = '{"msg": "use {} for templates", "count": 1}' + result = try_parse_json(text) + assert result is not None + assert result["msg"] == "use {} for templates" + assert result["count"] == 1 + + +def test_parse_json_embedded_with_braces_in_strings(): + text = 'Here is the result: {"msg": "pattern {x} works", "ok": true} done.' + result = try_parse_json(text) + assert result is not None + assert result["ok"] is True diff --git a/tests/test_synthesize.py b/tests/test_synthesize.py index 0a57895..682ffa9 100644 --- a/tests/test_synthesize.py +++ b/tests/test_synthesize.py @@ -85,3 +85,17 @@ async def test_consensus_strategy_keeps_analysis_oriented_prompt(): assert "extract the CONSENSUS" in RecordingBackend.last_prompt assert "DISAGREEMENTS:" in RecordingBackend.last_prompt assert "UNIQUE CLAIMS:" in RecordingBackend.last_prompt + + +async def test_consensus_does_not_mutate_backend_kwargs(): + gathered = make_gather(["answer A", "answer B"]) + kwargs: dict[str, object] = {"timeout": 30} + await synthesize(gathered, strategy="consensus", backend="recording", backend_kwargs=kwargs) + assert "model" not in kwargs + + +async def test_voting_does_not_mutate_backend_kwargs(): + gathered = make_gather(["answer A", "answer B"]) + kwargs: dict[str, object] = {"timeout": 30} + await synthesize(gathered, strategy="voting", backend="recording", backend_kwargs=kwargs) + assert "model" not in kwargs From 927152cf06beb4fcb3ab6041a242e946ecd3bac1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Apr 2026 05:33:15 +0000 Subject: [PATCH 2/2] Fix task validator: recommended_n > 10 should warn, not fail validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The validator treated recommended_n > 10 as a validation error with the text "Are you sure?" — which blocked legitimate task files while using language that suggested it should be a soft warning. Changed to emit a warnings.warn() instead, matching scatter.py's pattern for high n values. https://claude.ai/code/session_01H1AP8Drr8SE7QFevAookwL --- src/broadside_ai/task_validator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/broadside_ai/task_validator.py b/src/broadside_ai/task_validator.py index 0913fb1..17862eb 100644 --- a/src/broadside_ai/task_validator.py +++ b/src/broadside_ai/task_validator.py @@ -56,9 +56,12 @@ def validate_task_file(path: str) -> list[str]: if not isinstance(recommended_n, int) or recommended_n < 1: errors.append("meta.recommended_n must be a positive integer") elif recommended_n > 10: - errors.append( + import warnings + + warnings.warn( f"meta.recommended_n is {recommended_n}. " - "Performance plateaus at 3-5. Are you sure?" + "Performance typically plateaus at 3-5.", + stacklevel=2, ) return errors