Skip to content

fix(deriver): surface save_representation errors #660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mrlufepines wants to merge 1 commit into from
+14 −0
Closed

fix(deriver): surface save_representation errors #660

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/deriver/deriver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ async def process_representation_tasks_batch(
latest_message.created_at,
)

save_errors: list[str] = []
if observations.is_empty() or not message_ids:
logger.warning(
"Deriver generated zero observations for messages %s:%s in %s/%s!",
Expand Down Expand Up @@ -198,6 +199,7 @@ async def process_representation_tasks_batch(
logger.error(
"Failed to save representation for observer %s: %s", observer, e
)
save_errors.append(f"{observer}: {e.__class__.__name__}: {e}")

# Log metrics
overall_duration = (time.perf_counter() - overall_start) * 1000
Expand Down Expand Up @@ -252,3 +254,15 @@ async def process_representation_tasks_batch(
output_tokens=response.output_tokens,
)
)

# HERMES-PATCH 2026-05-03: surface save_representation failures to the
# queue manager so the queue row's `error` column is populated. Without
# this, transient backend errors (e.g. Gemini embedding quota) cause
# silent data loss: rows get processed=true with error=null. Raising
# after the metrics emit means partial successes still record metrics,
# but at least one observer's failure is now visible.
if save_errors:
raise RuntimeError(
f"save_representation failed for {len(save_errors)} observer(s): "
+ "; ".join(save_errors)
)
Comment on lines +264 to +268
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a project-specific exception instead of RuntimeError.

Raising a generic RuntimeError here loses domain semantics and violates the repo rule for typed exceptions in src/**. Please raise an appropriate custom exception from src/exceptions.py (e.g., a deriver/processing failure type) with the same aggregated message.

Suggested change 
-    if save_errors:
-        raise RuntimeError(
-            f"save_representation failed for {len(save_errors)} observer(s): "
-            + "; ".join(save_errors)
-        )
+    if save_errors:
+        raise DeriverProcessingException(
+            f"save_representation failed for {len(save_errors)} observer(s): "
+            + "; ".join(save_errors)
+        )

As per coding guidelines: src/**/*.py: “Use custom exceptions defined in src/exceptions.py with specific exception types.”

🤖 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/deriver/deriver.py` around lines 264 - 268, Replace the generic
RuntimeError raised when save_errors is non-empty with a project-specific
exception from src.exceptions.py: import the appropriate exception (e.g.,
DeriverProcessingError or another existing deriver/processing failure type) and
raise that instead with the same aggregated message built from save_errors;
update the raise in deriver.py (the block referencing save_errors and the
f"save_representation failed..." message) to use the imported custom exception
so the code follows the repo rule for typed exceptions.