feat(reasoning): expose mid-session style-swap detection via ReasoningAuditor (closes #370)#565
Conversation
|
Someone is attempting to deploy a commit to the sreerevanth's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds reasoning style fingerprinting to detect mid-session model swaps. Two new ChangesReasoning style fingerprint feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Test/Caller
participant Auditor as ReasoningAuditor
participant Fingerprint as fingerprint()/detect_mid_session_change()
participant Schema as ReasoningStyleFingerprint/StyleSwapAlert
Test->>Auditor: fingerprint_session(events, distance_threshold, split_ratio)
Auditor->>Fingerprint: fingerprint(events)
Fingerprint-->>Auditor: full StyleFingerprint
Auditor->>Fingerprint: detect_mid_session_change(events, split_ratio)
Fingerprint-->>Auditor: distance, threshold, detected
Auditor->>Auditor: _pydantic_fingerprint(fp, sample_size)
Auditor->>Schema: build ReasoningStyleFingerprint / StyleSwapAlert
Schema-->>Auditor: model instances
Auditor-->>Test: FingerprintReport(session_id, fingerprint, swap_alert)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🧪 PR Test Results
Python 3.12 · commit 19dbead |
|
Closes #370 |
…h#339) (sreerevanth#557) * feat(cli): add cost report command for spend by framework (sreerevanth#339) * fix(cli): warn on truncated results, skip malformed sessions, dedup group_by validation (sreerevanth#339)
…oning-fingerprint
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
agentwatch/core/schema.py (1)
247-275: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a
TYPE_CHECKING-guarded import for stricter typing.
from_dataclass(cls, other: object)usesobject+getattr(...)duck-typing, presumably to avoid a circular import withagentwatch.reasoning.fingerprint(which itself importsAgentEvent/EventTypefrom this module). If that's the reason, aTYPE_CHECKING-guarded import ofStyleFingerprintwould let you tighten the parameter type toStyleFingerprintwithout introducing a runtime circular import (assuming annotations are deferred in this file), improving type-checker coverage for this adapter.🤖 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 `@agentwatch/core/schema.py` around lines 247 - 275, The ReasoningStyleFingerprint.from_dataclass adapter is using object plus getattr duck-typing to sidestep a likely circular import, but this weakens type safety. Add a TYPE_CHECKING-guarded import for agentwatch.reasoning.fingerprint.StyleFingerprint and tighten from_dataclass’s parameter annotation from object to StyleFingerprint, relying on deferred annotations so the runtime import cycle is avoided while improving static checking.
🤖 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 `@agentwatch/reasoning/auditor.py`:
- Around line 161-162: The half-session fingerprint construction in auditor.py
is missing the event count, so first_half/second_half always end up with
sample_size set to 0. Update the fingerprint assembly in the code that builds
first_fp and second_fp inside the StyleSwapAlert path to pass the same
PLANNER_OUTPUT event count used by the full-session fingerprint, using the
existing fingerprint() and _pydantic_fingerprint() flow. Keep the fix aligned
with the surrounding alert-building logic so dashboards consuming
StyleSwapAlert.first_half and StyleSwapAlert.second_half see the correct
sample_size value.
- Around line 150-186: The `reason` logic in the fingerprint report is ambiguous
because `detect_mid_session_change()` returns `False, 0.0` both for missing
planner output and for genuinely identical halves. Update the `auditor.py` flow
around `detect_mid_session_change`, `FingerprintReport`, and `StyleSwapAlert` so
that “insufficient_planner_signal” is derived from a reliable signal such as
half-level `sample_size`/planner counts, and reserve “below_threshold” for real
zero-distance or low-distance comparisons. This keeps the diagnostic `reason`
accurate when `distance == 0.0` but both halves still have planner events.
In `@tests/test_reasoning_style_fingerprint.py`:
- Around line 139-153: The 5-event parametrized case in
test_fingerprint_session_handles_edge_cases is misleading because
fingerprint_session short-circuits on insufficient_events before any
below-threshold logic runs. Update the case using _plan and ReasoningAuditor so
it uses 6+ events if you intend to cover the below_threshold branch, and assert
report.swap_alert.reason == "below_threshold" for that row; otherwise, change
the inline comment to match the insufficient_events boundary behavior.
---
Nitpick comments:
In `@agentwatch/core/schema.py`:
- Around line 247-275: The ReasoningStyleFingerprint.from_dataclass adapter is
using object plus getattr duck-typing to sidestep a likely circular import, but
this weakens type safety. Add a TYPE_CHECKING-guarded import for
agentwatch.reasoning.fingerprint.StyleFingerprint and tighten from_dataclass’s
parameter annotation from object to StyleFingerprint, relying on deferred
annotations so the runtime import cycle is avoided while improving static
checking.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 782d5620-8bf2-4f58-92cc-fa21e811ff9b
📒 Files selected for processing (4)
CHANGELOG.mdagentwatch/core/schema.pyagentwatch/reasoning/auditor.pytests/test_reasoning_style_fingerprint.py
| full = fingerprint(events) | ||
| detected, distance = detect_mid_session_change( | ||
| events, | ||
| split_ratio=split_ratio, | ||
| distance_threshold=distance_threshold, | ||
| ) | ||
| # Compute both halves for diagnostic context, even when we cannot | ||
| # actually compare them — this lets downstream consumers see *why* | ||
| # detection returned ``False``. | ||
| cut = int(len(events) * split_ratio) | ||
| first, second = events[:cut], events[cut:] | ||
| first_fp = self._pydantic_fingerprint(fingerprint(first)) | ||
| second_fp = self._pydantic_fingerprint(fingerprint(second)) | ||
| session_id = events[0].session_id if events else "" | ||
| reason: str | None = None | ||
| if not detected: | ||
| if len(events) < 6: | ||
| reason = "insufficient_events" | ||
| elif distance == 0.0: | ||
| reason = "insufficient_planner_signal" | ||
| else: | ||
| reason = "below_threshold" | ||
| plans_count = sum(1 for e in events if e.event_type == EventType.PLANNER_OUTPUT) | ||
| fp_pydantic = self._pydantic_fingerprint(full, sample_size=plans_count) | ||
| return FingerprintReport( | ||
| session_id=session_id, | ||
| fingerprint=fp_pydantic, | ||
| swap_alert=StyleSwapAlert( | ||
| session_id=session_id, | ||
| detected=detected, | ||
| distance=round(distance, 4), | ||
| threshold=distance_threshold, | ||
| first_half=first_fp, | ||
| second_half=second_fp, | ||
| reason=reason, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
reason derivation conflates "no planner signal" with a genuine zero-distance "below threshold".
detect_mid_session_change short-circuits to (False, 0.0) both when a half lacks PLANNER_OUTPUT events and when two halves happen to be identical (real distance of 0.0). Here, elif distance == 0.0: reason = "insufficient_planner_signal" treats both cases identically, so a session with genuinely identical planner style across halves is mislabeled as "insufficient signal" instead of "below_threshold". The test test_fingerprint_session_no_swap_detected_consistent_plans even asserts reason in {"insufficient_planner_signal", "below_threshold"}, acknowledging this ambiguity rather than resolving it. Since dashboards will render this reason field, an inaccurate label undermines the diagnostic value this PR is meant to add.
This also duplicates fingerprint(first)/fingerprint(second) computation that detect_mid_session_change already performs internally.
A low-effort fix: populate sample_size on the half fingerprints (see next comment) and use that as the reliable "no planner output" signal instead of proxying via distance == 0.0.
🐛 Proposed fix
cut = int(len(events) * split_ratio)
first, second = events[:cut], events[cut:]
- first_fp = self._pydantic_fingerprint(fingerprint(first))
- second_fp = self._pydantic_fingerprint(fingerprint(second))
+ first_plans = sum(1 for e in first if e.event_type == EventType.PLANNER_OUTPUT)
+ second_plans = sum(1 for e in second if e.event_type == EventType.PLANNER_OUTPUT)
+ first_fp = self._pydantic_fingerprint(fingerprint(first), sample_size=first_plans)
+ second_fp = self._pydantic_fingerprint(fingerprint(second), sample_size=second_plans)
session_id = events[0].session_id if events else ""
reason: str | None = None
if not detected:
if len(events) < 6:
reason = "insufficient_events"
- elif distance == 0.0:
+ elif first_plans == 0 or second_plans == 0:
reason = "insufficient_planner_signal"
else:
reason = "below_threshold"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| full = fingerprint(events) | |
| detected, distance = detect_mid_session_change( | |
| events, | |
| split_ratio=split_ratio, | |
| distance_threshold=distance_threshold, | |
| ) | |
| # Compute both halves for diagnostic context, even when we cannot | |
| # actually compare them — this lets downstream consumers see *why* | |
| # detection returned ``False``. | |
| cut = int(len(events) * split_ratio) | |
| first, second = events[:cut], events[cut:] | |
| first_fp = self._pydantic_fingerprint(fingerprint(first)) | |
| second_fp = self._pydantic_fingerprint(fingerprint(second)) | |
| session_id = events[0].session_id if events else "" | |
| reason: str | None = None | |
| if not detected: | |
| if len(events) < 6: | |
| reason = "insufficient_events" | |
| elif distance == 0.0: | |
| reason = "insufficient_planner_signal" | |
| else: | |
| reason = "below_threshold" | |
| plans_count = sum(1 for e in events if e.event_type == EventType.PLANNER_OUTPUT) | |
| fp_pydantic = self._pydantic_fingerprint(full, sample_size=plans_count) | |
| return FingerprintReport( | |
| session_id=session_id, | |
| fingerprint=fp_pydantic, | |
| swap_alert=StyleSwapAlert( | |
| session_id=session_id, | |
| detected=detected, | |
| distance=round(distance, 4), | |
| threshold=distance_threshold, | |
| first_half=first_fp, | |
| second_half=second_fp, | |
| reason=reason, | |
| ), | |
| ) | |
| full = fingerprint(events) | |
| detected, distance = detect_mid_session_change( | |
| events, | |
| split_ratio=split_ratio, | |
| distance_threshold=distance_threshold, | |
| ) | |
| # Compute both halves for diagnostic context, even when we cannot | |
| # actually compare them — this lets downstream consumers see *why* | |
| # detection returned ``False``. | |
| cut = int(len(events) * split_ratio) | |
| first, second = events[:cut], events[cut:] | |
| first_plans = sum(1 for e in first if e.event_type == EventType.PLANNER_OUTPUT) | |
| second_plans = sum(1 for e in second if e.event_type == EventType.PLANNER_OUTPUT) | |
| first_fp = self._pydantic_fingerprint(fingerprint(first), sample_size=first_plans) | |
| second_fp = self._pydantic_fingerprint(fingerprint(second), sample_size=second_plans) | |
| session_id = events[0].session_id if events else "" | |
| reason: str | None = None | |
| if not detected: | |
| if len(events) < 6: | |
| reason = "insufficient_events" | |
| elif first_plans == 0 or second_plans == 0: | |
| reason = "insufficient_planner_signal" | |
| else: | |
| reason = "below_threshold" | |
| plans_count = sum(1 for e in events if e.event_type == EventType.PLANNER_OUTPUT) | |
| fp_pydantic = self._pydantic_fingerprint(full, sample_size=plans_count) | |
| return FingerprintReport( | |
| session_id=session_id, | |
| fingerprint=fp_pydantic, | |
| swap_alert=StyleSwapAlert( | |
| session_id=session_id, | |
| detected=detected, | |
| distance=round(distance, 4), | |
| threshold=distance_threshold, | |
| first_half=first_fp, | |
| second_half=second_fp, | |
| reason=reason, | |
| ), | |
| ) |
🤖 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 `@agentwatch/reasoning/auditor.py` around lines 150 - 186, The `reason` logic
in the fingerprint report is ambiguous because `detect_mid_session_change()`
returns `False, 0.0` both for missing planner output and for genuinely identical
halves. Update the `auditor.py` flow around `detect_mid_session_change`,
`FingerprintReport`, and `StyleSwapAlert` so that “insufficient_planner_signal”
is derived from a reliable signal such as half-level `sample_size`/planner
counts, and reserve “below_threshold” for real zero-distance or low-distance
comparisons. This keeps the diagnostic `reason` accurate when `distance == 0.0`
but both halves still have planner events.
| first_fp = self._pydantic_fingerprint(fingerprint(first)) | ||
| second_fp = self._pydantic_fingerprint(fingerprint(second)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Half fingerprints always report sample_size == 0.
first_fp/second_fp are built via _pydantic_fingerprint(fingerprint(first)) without passing sample_size, unlike the full-session fingerprint (line 173). Consumers of StyleSwapAlert.first_half/second_half (e.g. dashboards) will always see sample_size: 0 for both halves, even though the field is documented as "number of PLANNER_OUTPUT events used". See the proposed fix in the adjacent comment on lines 150-186, which addresses this alongside the reason ambiguity.
🤖 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 `@agentwatch/reasoning/auditor.py` around lines 161 - 162, The half-session
fingerprint construction in auditor.py is missing the event count, so
first_half/second_half always end up with sample_size set to 0. Update the
fingerprint assembly in the code that builds first_fp and second_fp inside the
StyleSwapAlert path to pass the same PLANNER_OUTPUT event count used by the
full-session fingerprint, using the existing fingerprint() and
_pydantic_fingerprint() flow. Keep the fix aligned with the surrounding
alert-building logic so dashboards consuming StyleSwapAlert.first_half and
StyleSwapAlert.second_half see the correct sample_size value.
| @pytest.mark.parametrize( | ||
| "events, expected_reason", | ||
| [ | ||
| ([], "insufficient_events"), | ||
| ([_plan("x", 0)], "insufficient_events"), | ||
| ([_plan("plan", i) for i in range(5)], None), # below threshold in even dist | ||
| ], | ||
| ) | ||
| def test_fingerprint_session_handles_edge_cases(events, expected_reason): | ||
| auditor = ReasoningAuditor() | ||
| report = auditor.fingerprint_session(events) | ||
| if expected_reason is None: | ||
| assert report.swap_alert.detected is False | ||
| else: | ||
| assert report.swap_alert.reason == expected_reason |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Misleading comment / weak assertion for the 5-event parametrized case.
[_plan("plan", i) for i in range(5)] has only 5 events, which is < 6, so fingerprint_session will always short-circuit to reason = "insufficient_events" (line 166 in auditor.py) — never reaching a "below threshold" branch as the inline comment # below threshold in even dist implies. The test also doesn't assert reason for this row (only detected is False), so the mismatch goes unnoticed. Consider bumping this case to 6+ events and asserting reason == "below_threshold" to actually cover that path, or correcting the comment if the intent was just to exercise the "insufficient_events" boundary again.
🐛 Proposed fix
([], "insufficient_events"),
([_plan("x", 0)], "insufficient_events"),
- ([_plan("plan", i) for i in range(5)], None), # below threshold in even dist
+ ([_plan("plan", i) for i in range(6)], "below_threshold"),
],
)
def test_fingerprint_session_handles_edge_cases(events, expected_reason):
auditor = ReasoningAuditor()
report = auditor.fingerprint_session(events)
- if expected_reason is None:
- assert report.swap_alert.detected is False
- else:
- assert report.swap_alert.reason == expected_reason
+ assert report.swap_alert.detected is False
+ assert report.swap_alert.reason == expected_reason📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.mark.parametrize( | |
| "events, expected_reason", | |
| [ | |
| ([], "insufficient_events"), | |
| ([_plan("x", 0)], "insufficient_events"), | |
| ([_plan("plan", i) for i in range(5)], None), # below threshold in even dist | |
| ], | |
| ) | |
| def test_fingerprint_session_handles_edge_cases(events, expected_reason): | |
| auditor = ReasoningAuditor() | |
| report = auditor.fingerprint_session(events) | |
| if expected_reason is None: | |
| assert report.swap_alert.detected is False | |
| else: | |
| assert report.swap_alert.reason == expected_reason | |
| `@pytest.mark.parametrize`( | |
| "events, expected_reason", | |
| [ | |
| ([], "insufficient_events"), | |
| ([_plan("x", 0)], "insufficient_events"), | |
| ([_plan("plan", i) for i in range(6)], "below_threshold"), | |
| ], | |
| ) | |
| def test_fingerprint_session_handles_edge_cases(events, expected_reason): | |
| auditor = ReasoningAuditor() | |
| report = auditor.fingerprint_session(events) | |
| assert report.swap_alert.detected is False | |
| assert report.swap_alert.reason == expected_reason |
🤖 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 `@tests/test_reasoning_style_fingerprint.py` around lines 139 - 153, The
5-event parametrized case in test_fingerprint_session_handles_edge_cases is
misleading because fingerprint_session short-circuits on insufficient_events
before any below-threshold logic runs. Update the case using _plan and
ReasoningAuditor so it uses 6+ events if you intend to cover the below_threshold
branch, and assert report.swap_alert.reason == "below_threshold" for that row;
otherwise, change the inline comment to match the insufficient_events boundary
behavior.
sreerevanth
left a comment
sreerevanth
left a comment
There was a problem hiding this comment.
Great work overall—this is a useful addition and the implementation looks solid. Before merging, could you address the two CodeRabbit findings?
Populate sample_size for the first/second-half fingerprints.
Differentiate "insufficient_planner_signal" from a genuine zero-distance (below_threshold) case instead of relying on distance == 0.0.
After those are fixed, I'm happy to merge.
|
Hey @sreerevanth, this PR has been ready for review for a while -- offering this friendly bump. The implementation matches the issue's specified behavior and CI is passing. PR: #565 When you have a moment, a review would be much appreciated. I am happy to iterate on any feedback. If the timing is not right, no worries at all -- just a gentle nudge from a contributor who has been waiting patiently. � |
3 participants
Summary
Closes #370 — exposes the existing ingerprint() and detect_mid_session_change() helpers through the public ReasoningAuditor API and adds first-class schema models so the chain can be persisted and rendered in dashboards.
Motivation
RSN-008 ships a fingerprint primitive inside �gentwatch.reasoning.fingerprint, but it is not reachable from the auditor's public surface, and there is no schema entry to persist the swap detection — so downstream consumers cannot subscribe to it. This PR wires the two together.
Changes
eason blob for short-circuit cases.
eason is auto-derived for non-detections: insufficient_events, insufficient_planner_signal, or �elow_threshold.
Regression Safety
detect_mid_session_change() keeps its all-zeros sentinel rejection (#538 regression tests still pass — all 5 guards green). Fingerprints with one planner-empty half still report detected=False with a
eason instead of a false-positive swap alarm.
Checklist
ruff checkandruff format --checkare cleanSummary by CodeRabbit
New Features
Bug Fixes