fix(reviewer): success=False when CharacterAnimationReviewer finds issues

The contract test in test_character_animation_pipeline.py asserts
result.success after the QA run, implying success should reflect
whether QA passed. But CharacterAnimationReviewer always returned
success=True even when issues were found and status was 'revise'.

This creates a silent failure path: callers that gate on result.success
(the standard ToolResult contract) would treat a broken rig as a clean
pass without ever reading report['status'].

Fix: return success=not issues so result.success is False when QA finds
problems, consistent with the test contract and with how execution errors
are already handled (success=False on exception paths).

Also adds test_character_reviewer_success_false_when_qa_finds_issues to
explicitly assert this contract — a broken rig with missing joints must
produce status='revise', non-empty issues[], and success=False.

tests/contracts/test_character_animation_pipeline.py

@@ -186,6 +186,35 @@ def test_character_animation_smoke_flow(tmp_path):
     assert qa_report["checks"]["schema_valid"] is True
 
 
+def test_character_reviewer_success_false_when_qa_finds_issues():
+    """
+    CharacterAnimationReviewer.success must be False when QA finds issues.
+
+    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>.
+    """
+    result = CharacterAnimationReviewer().execute(
+        {
+            # Minimal rig_plan with missing joints — will trigger schema issues
+            "rig_plan": {"characters": [{"id": "char1", "role": "lead"}]},
+            "pose_library": {},
+            "action_timeline": {},
+            "review_level": "static",
+        }
+    )
+
+    qa_report = result.data["character_qa_report"]
+    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):
     result = CharacterSpecGenerator().execute(
         {

tools/character/character_animation.py

@@ -888,8 +888,12 @@ 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=True,
+            success=not issues,
             data={"character_qa_report": report},
             artifacts=artifacts,
             duration_seconds=round(time.time() - start, 2),