fix(anthropic): preserve valid complete responses

[PierrunoYT]

Summary

• preserve valid non-streaming Anthropic content when usage metadata is missing or malformed
• serialize tool inputs with valid JSON escaping and reject non-object tool arguments
• isolate notification environment tests to remove a full-suite race found during validation

Testing

• cargo test llm::anthropic::tests --locked -- --nocapture
• cargo test json::tests --locked -- --nocapture
• cargo test --locked (232 passed)
• cargo check --locked
• cargo build --locked
• git diff --check

Follow-up to #40.

Fixes #46.

Summary by CodeRabbit

• Bug Fixes

  • Improved JSON escaping for strings and object keys, including control characters.
  • Added support for backspace and form-feed escape sequences when parsing JSON.
  • Preserved escaped tool inputs correctly and rejected invalid non-object tool inputs.
  • Made response usage parsing more resilient by defaulting unavailable token counts to zero.

• Tests

  • Added coverage for JSON round-tripping and malformed or incomplete response data.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes improve JSON escaping and parsing, make non-streaming Anthropic response handling tolerant of incomplete usage metadata while validating tool inputs, and isolate notification tests that modify CAIRN_SOUND.

Changes

JSON serialization and parsing

Layer / File(s)	Summary
Escaped JSON strings and roundtrip validation src/json.rs	String values and object keys are serialized with JSON escaping, \b and \f escapes are parsed, and escaped key/value roundtripping is tested.

Anthropic response parsing

Layer / File(s)	Summary
Tool input validation and usage defaults src/llm/anthropic.rs	tool_use.input must be an object, while missing or malformed usage counters independently default to 0; tests cover both behaviors and escaped inputs.

Notification test isolation

Layer / File(s)	Summary
Environment isolation for notification tests src/notify.rs	Tests synchronize access to CAIRN_SOUND and restore its previous value through an RAII guard.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• Cairn/cairn-code#40: Both changes modify non-streaming Anthropic response parsing, including tool input validation and usage token handling.

Suggested reviewers: euxaristia

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is clearly related to the Anthropic response-handling fix, though it doesn't mention the JSON escaping and test isolation changes.
Linked Issues check	✅ Passed	The PR implements the requirements in #46: optional usage defaults, escaped tool JSON, non-object rejection, and isolated CAIRN_SOUND tests.
Out of Scope Changes check	✅ Passed	No clear out-of-scope code changes are evident; the touched JSON, Anthropic, and notify test updates all support the linked issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/notify.rs (1)

148-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restore the default-behavior assertion.

enabled_defaults_true_without_env no longer tests its named contract. Remove CAIRN_SOUND under the guard and assert enabled() is true before testing the explicit "0", "false", and "1" values.

🤖 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/notify.rs` around lines 148 - 158, Update
enabled_defaults_true_without_env to remove CAIRN_SOUND while ENV_LOCK is held,
then assert enabled() returns true before setting and validating the explicit
"0", "false", and "1" values. Preserve the existing SoundEnv guard and
explicit-value assertions.

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/notify.rs`:
- Line 149: Update the test guards acquiring ENV_LOCK at both locations to
recover from mutex poisoning by using the poisoned guard’s inner value instead
of calling unwrap directly. Preserve the existing guard lifetime and environment
restoration behavior so subsequent tests can run after a prior panic.

---

Outside diff comments:
In `@src/notify.rs`:
- Around line 148-158: Update enabled_defaults_true_without_env to remove
CAIRN_SOUND while ENV_LOCK is held, then assert enabled() returns true before
setting and validating the explicit "0", "false", and "1" values. Preserve the
existing SoundEnv guard and explicit-value assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1be713e6-0999-4acd-9f42-6ef64eaa4d9e

📥 Commits

Reviewing files that changed from the base of the PR and between 68a2601 and fdc7fea.

📒 Files selected for processing (3)

• src/json.rs
• src/llm/anthropic.rs
• src/notify.rs

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Recover from mutex poisoning in these test guards.

If either test panics after acquiring ENV_LOCK, the mutex becomes poisoned and the other test will panic at .unwrap() before restoring or testing its environment. Use unwrap_or_else(|error| error.into_inner()) so one failed assertion does not cascade into unrelated test failures.

Also applies to: 163-163

🤖 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/notify.rs` at line 149, Update the test guards acquiring ENV_LOCK at both
locations to recover from mutex poisoning by using the poisoned guard’s inner
value instead of calling unwrap directly. Preserve the existing guard lifetime
and environment restoration behavior so subsequent tests can run after a prior
panic.

[euxaristia]

LGTM! Approved after audit.