fix(character-reviewer): align success contract with visual_qa pattern

Per maintainer feedback on PR #227:
- Revert success=not issues back to success=True — tool execution
  succeeded even when QA finds issues; verdict lives in status/issues
- Update test to assert the real contract: success=True + status='revise'
  + issues non-empty, matching how compose-director actually gates
- Consistent with visual_qa.py: success=True, verdict in validation_passed

tests/contracts/test_character_animation_pipeline.py

@@ -188,11 +188,12 @@ def test_character_animation_smoke_flow(tmp_path):
 
 def test_character_reviewer_success_false_when_qa_finds_issues():
     """
-    CharacterAnimationReviewer.success must be False when QA finds issues.
+    CharacterAnimationReviewer surfaces QA failures via status/issues, not success.
 
-    Callers such as the compose-director gate on result.success to decide
-    whether to proceed. If success is always True, a broken rig silently
-    passes the gate. See PR #166 (closed) and the follow-up fix in #<this PR>.
+    success=True means the tool executed successfully; the QA verdict lives in
+    character_qa_report.status and character_qa_report.issues — matching the
+    pattern used by visual_qa.py (success=True, verdict in validation_passed).
+    compose-director gates on report.status, not result.success.
     """
     result = CharacterAnimationReviewer().execute(
         {
@@ -205,14 +206,11 @@ def test_character_reviewer_success_false_when_qa_finds_issues():
     )
 
     qa_report = result.data["character_qa_report"]
+    assert result.success is True, "tool execution must succeed even when QA finds issues"
     assert qa_report["status"] == "revise", (
         f"Expected status='revise' for a broken rig, got '{qa_report['status']}'"
     )
     assert len(qa_report["issues"]) > 0, "Expected at least one issue for a broken rig"
-    assert result.success is False, (
-        "success must be False when QA finds issues — "
-        "callers gate on result.success without inspecting report['status']"
-    )
 
 
 def test_character_style_is_normalized_for_schema(tmp_path):

tools/character/character_animation.py

@@ -888,12 +888,8 @@ class CharacterAnimationReviewer(BaseTool):
             },
         }
         artifacts = _write_json(inputs.get("output_path"), report)
-        # success=False when QA finds issues so callers can gate on result.success
-        # without needing to inspect report["status"]. This mirrors the contract
-        # asserted in tests/contracts/test_character_animation_pipeline.py and
-        # is consistent with how visual_qa.py surfaces validation failures.
         return ToolResult(
-            success=not issues,
+            success=True,
             data={"character_qa_report": report},
             artifacts=artifacts,
             duration_seconds=round(time.time() - start, 2),