-
Notifications
You must be signed in to change notification settings - Fork 0
feat+test: add drift detection in record_decision and 57 unit tests for FrameTracer/DivergenceDetector #209
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
5a00117
f380508
8020e78
acdacd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,8 +98,9 @@ async def record_decision( | |
| self, | ||
| reasoning: str, | ||
| confidence: float, | ||
| evidence: list[dict[str, Any]], | ||
| chosen_action: str, | ||
| *, | ||
| evidence: list[dict[str, Any]] | None = None, | ||
| evidence_event_ids: list[str] | None = None, | ||
| upstream_event_ids: list[str] | None = None, | ||
| alternatives: list[dict[str, Any]] | None = None, | ||
|
|
@@ -114,14 +115,42 @@ async def record_decision( | |
| name=name, | ||
| reasoning=reasoning, | ||
| confidence=max(0.0, min(1.0, confidence)), | ||
| evidence=evidence, | ||
| evidence=evidence or [], | ||
| evidence_event_ids=evidence_event_ids or [], | ||
| alternatives=alternatives or [], | ||
| chosen_action=chosen_action, | ||
| importance=0.7, | ||
| upstream_event_ids=upstream_event_ids or [], | ||
| ) | ||
| await self._emit_event(event) | ||
|
|
||
| # Detect drift against the original execution if a detector is active | ||
| drift_detector = getattr(self, "_drift_detector", None) | ||
| if drift_detector is not None: | ||
| drift_index = getattr(self, "_drift_compare_index", 0) | ||
| original_events = getattr(drift_detector, "original_events", []) | ||
| # Advance to the current decision event before comparing, skipping non-decision events | ||
| while drift_index < len(original_events) and original_events[drift_index].get("event_type") != "decision": | ||
| drift_index += 1 | ||
| event_dict = { | ||
| "event_type": "decision", | ||
| "data": { | ||
| "chosen_action": chosen_action, | ||
| "action": chosen_action, | ||
| "confidence": event.confidence, | ||
| }, | ||
| } | ||
| drift = drift_detector.compare(event_dict, drift_index) | ||
| # Advance to the next decision event in the baseline, skipping non-decision events | ||
| next_index = drift_index + 1 | ||
| while next_index < len(original_events) and original_events[next_index].get("event_type") != "decision": | ||
| next_index += 1 | ||
| self._drift_compare_index = next_index | ||
|
Comment on lines
+130
to
+148
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in acdacd5. Before calling |
||
| if drift is not None: | ||
| drift_events_list = getattr(self, "_drift_events", None) | ||
| if drift_events_list is not None: | ||
| drift_events_list.append(drift) | ||
|
|
||
| return event.id | ||
|
|
||
| async def record_tool_call( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged. The SDK behavior changes (
evidencekeyword-only + drift detection inrecord_decision) are prerequisites for the tests to exercise correctly — the tests validate this runtime behavior. The PR description has been noted; if desired, the title can be updated to reflect the dual scope (runtime fix + test coverage).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR title and description updated to reflect the dual scope: the SDK behavior changes (keyword-only
evidence, drift detection inrecord_decision, drift state init onrestore()) are now called out explicitly alongside the unit tests.