feat(analytics): enhance error categorization and enrich analytics events for tools - #232
Merged
Merged
Conversation
Contributor
WalkthroughThe change adds coarse analytics error categories, enriches tool events with normalized outcome data, prevents duplicate final events, and classifies failures in agent and crawl execution paths. ChangesAnalytics outcome reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ToolCaller
participant defineTool
participant ErrorClassifier
participant AnalyticsHelper
ToolCaller->>defineTool: Execute tool
defineTool->>ErrorClassifier: Classify thrown error
ErrorClassifier-->>defineTool: Return analytics category
defineTool->>AnalyticsHelper: Emit enriched success or failure event
defineTool-->>ToolCaller: Return result or rethrow error
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/lib/define-tool.ts`:
- Around line 257-261: Finalize success analytics only after all invocation work
succeeds: in src/lib/define-tool.ts lines 257-261, call def.format before the
default analytics emission; in test/lib/define-tool.spec.ts lines 170-182, add
coverage for formatter failure with default successful analytics properties; in
src/tools/crawl.ts lines 365-370, move the terminal crawl event emission until
after page content fetching and result construction complete.
In `@src/lib/error-classifier.ts`:
- Around line 161-171: Unify HTTP 4xx analytics classification by extracting the
shared status-to-category policy from ANALYTICS_CATEGORY and the mapper in
define-tool.ts into one reusable mapper. Update both the error classification
path and the status mapping path to use it, ensuring UNAUTHORIZED, FORBIDDEN,
and NOT_FOUND resolve consistently as user_error.
In `@src/tools/agent.ts`:
- Around line 617-619: Ensure every failure path in the agent flow assigns a
concrete analytics category before the self-emission block builds
`error_category`, including the getOrCreateSession catch and
normalizeUploadCommand errors. Update the relevant handlers around
getOrCreateSession, normalizeUploadCommand, and the outer failure emitter so
caught errors are categorized or passed through a centralized failure emitter,
preventing the defineTool latch from receiving an unknown category.
🪄 Autofix
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
Run ID: 117332f7-04b0-413d-af39-ec75e0c6d8bb
📒 Files selected for processing (8)
src/@types/types.d.tssrc/lib/define-tool.tssrc/lib/error-classifier.tssrc/tools/agent.tssrc/tools/crawl.tstest/lib/define-tool.spec.tstest/tools/agent.spec.tstest/tools/crawl.spec.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/lib/error-classifier.spec.ts (1)
170-198: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPin the complete analytics normalization contract in these tests.
The HTTP loop uses
categoryFromStatus(status)as its expected value. IfcategoryFromStatusandtoAnalyticsCategoryregress together, the test can still pass. Assert the concrete'user_error'and'api_error'values.Add assertions for
NAVIGATION_FAILED,INVALID_PARAMS, andUNKNOWN. TheseErrorCategoryvalues are not covered by the current non-HTTP test.🤖 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 `@test/lib/error-classifier.spec.ts` around lines 170 - 198, Update the analytics category tests in the “analytics categories” suite to assert concrete HTTP outputs of user_error for 401/403/404 and api_error for 500, rather than comparing to categoryFromStatus. Extend the non-HTTP coverage in the toAnalyticsCategory test with NAVIGATION_FAILED, INVALID_PARAMS, and UNKNOWN, asserting their expected normalized categories.
🤖 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.
Nitpick comments:
In `@test/lib/error-classifier.spec.ts`:
- Around line 170-198: Update the analytics category tests in the “analytics
categories” suite to assert concrete HTTP outputs of user_error for 401/403/404
and api_error for 500, rather than comparing to categoryFromStatus. Extend the
non-HTTP coverage in the toAnalyticsCategory test with NAVIGATION_FAILED,
INVALID_PARAMS, and UNKNOWN, asserting their expected normalized categories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9641b26-851f-490d-b573-7b6dbbbceafb
📒 Files selected for processing (6)
src/lib/define-tool.tssrc/lib/error-classifier.tssrc/tools/agent.tssrc/tools/crawl.tstest/lib/define-tool.spec.tstest/lib/error-classifier.spec.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- test/lib/define-tool.spec.ts
- src/tools/agent.ts
- src/tools/crawl.ts
- src/lib/error-classifier.ts
- src/lib/define-tool.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes