feat(search): emit outcome telemetry for search invocations (ENT-1938) - #2130
Open
evisdren wants to merge 4 commits into
Open
feat(search): emit outcome telemetry for search invocations (ENT-1938)#2130evisdren wants to merge 4 commits into
evisdren wants to merge 4 commits into
Conversation
Add a cli_search_completed event carrying success, error_class, result_count, and duration_ms for entire search and entire checkpoint search (both the checkpoint and --code paths). Client-side failures — auth, jurisdiction cell skips, gateway-without-query-serve — never reach a server, so the CLI is the only place they can be measured. Error classes come from typed errors, never message matching. To make the chain classifiable: loginHintErr and the all-cells-skipped path now preserve their typed cause behind the unchanged user-facing message (hintError), and semantic-search non-OK responses return a typed search.HTTPStatusError instead of an untyped fmt.Errorf. Content-free by construction: booleans, enums, counts, durations only. Gated on the telemetry opt-in setting and ENTIRE_TELEMETRY_OPTOUT via the detached PostHog path. Interactive TUI re-searches are not emitted; the initial fetch is measured before the TUI starts so duration never includes TUI time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01M0Y4Z4KW8N1GK3TMA3KGV9ZW
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit cd05c83. Configure here.
| // cause in the chain: the two "region" variants (gateway without | ||
| // query-serve vs. client-side jurisdiction skip) are different | ||
| // failures and telemetry classifies them from the typed cause. | ||
| lastErr = &hintError{msg: errNoRegionAvailable.Error(), errs: []error{errNoRegionAvailable, skipErr}} |
There was a problem hiding this comment.
Mixed skip causes misclassified
Medium Severity
When every cell is skipped and the causes mix auth.ErrNoCellForJurisdiction and search.ErrCellUnavailable, only the last cell’s error is kept in the hintError chain. classifySearchError then reports whichever class that last cause maps to, so the same multi-cell failure can emit cell_skip or region_unavailable depending on cell order—undermining the distinction this change is meant to preserve.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit cd05c83. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in, content-free PostHog telemetry for search outcomes so the CLI can distinguish “search invoked” from “search succeeded/failed” (ENT-1938), without logging query text or results content. It threads typed errors through existing search paths so failures can be classified via errors.Is/As instead of message matching.
Changes:
- Introduces a new
cli_search_completedevent emitted once per executed search request (semantic checkpoint search and--codesearch) with duration, success, and either result count or a coarse typed error class. - Adds
search.HTTPStatusErrorto preserve existing user-facing error wording while exposing HTTP status for typed classification. - Adds a
hintErrorwrapper to preserve user-facing hint strings while keeping underlying typed causes matchable for telemetry classification.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/telemetry/search_outcome.go | Defines SearchOutcome and builds/sends the new cli_search_completed event via detached telemetry. |
| cmd/entire/cli/telemetry/search_outcome_test.go | Tests event payload shape (success vs failure fields). |
| cmd/entire/cli/search/search.go | Introduces typed HTTPStatusError for non-OK semantic search responses. |
| cmd/entire/cli/search/search_test.go | Verifies semantic search non-OK responses return *HTTPStatusError with status code preserved. |
| cmd/entire/cli/search_v4.go | Adds hintError wrapper and preserves typed causes in the “all semantic cells skipped” path. |
| cmd/entire/cli/search_telemetry.go | Implements classifySearchError and emitSearchOutcome to record the new outcome event when telemetry is enabled. |
| cmd/entire/cli/search_telemetry_test.go | Table-tests error classification and validates hintError preserves message + errors.Is chain. |
| cmd/entire/cli/search_cmd.go | Emits outcome telemetry for both semantic search and code search, timing only the request (excluding TUI time). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+67
to
+71
| func emitSearchOutcome(ctx context.Context, cmd *cobra.Command, mode string, resultCount int, duration time.Duration, err error) { | ||
| s, loadErr := LoadEntireSettings(ctx) | ||
| if loadErr != nil || !s.IsTelemetryEnabled() { | ||
| return | ||
| } |
Comment on lines
390
to
394
| switch { | ||
| case errors.Is(r.err, search.ErrCellUnavailable), errors.Is(r.err, auth.ErrNoCellForJurisdiction): | ||
| skipped = append(skipped, r.group.label()) | ||
| skipErr = r.err | ||
| case errors.Is(r.err, search.ErrRepoFilterUnmatched): |
Review findings applied: - Classify network failures via the existing isRecapNetworkError instead of a raw net.Error check, so a user's Ctrl-C (context.Canceled inside url.Error) is "other", never "network" — the raw check inflated the exact rate the event exists to measure. Timeouts stay "network". - Keep every per-cell skip cause in the all-cells-skipped error, not just the last one, so a mixed-cause fan-out classifies by the classifier's precedence instead of cell iteration order. New end-to-end test pins classifySemanticCells → classifySearchError including mixed causes. - Drop the derivable SearchOutcome.Success field (success == empty ErrorClass, enforced by construction instead of convention). - Add coverage_incomplete to successful payloads so a degraded success (failed regions, skipped repos, truncated index) is distinguishable from a genuine zero-result search. - Check ENTIRE_TELEMETRY_OPTOUT before the settings load so opted-out users pay nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01M0Y6B8YAGVG9K84WPXNV3CZR
Trail Review finding: parseSearchResponse returned plain fmt.Errorf for a 200 with undecodable JSON or an application-level error field, so those service failures classified as "other" while the same bugs surfaced with a proper status code classified as "server". Both shapes now return a typed search.MalformedResponseError (message wording unchanged) and the classifier maps it to the server class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01M0Y7CXG0K9R0EKEVEAYYSG6D
Trail Review finding: emitSearchOutcome only fired for the one-shot initial request, so TUI re-searches, pagination, the code tab, and bare `entire search` (straight to TUI) never emitted cli_search_completed — a large share of real invocations was invisible. Instrument the seams instead of the call sites: the semanticSearcher is wrapped once per invocation (instrumentSemanticSearcher), and searchAllCells emits for itself, so every entry point — one-shot, TUI initial fetch, re-searches, pagination, code tab — is covered by construction and new entry points can't silently drop off. Each timer wraps only the request, so TUI dwell time is still never counted. The command path travels via the wrapper closure and codeSearchOpts because the TUI searches long after the command layer returns. TUI searches fire per submitted query (Enter), not per keystroke, so event volume stays one event per real search request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01M0Y8JN191FD8SXA4KK4M6XDT
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
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.
https://entire.io/gh/entireio/cli/trails/1146
Why
The tester audit (ENT-1938) found 88.5% of no-value search sessions failed to obtain a usable response at all, dominated by client-side auth/configuration failures that never reach a server.
cli_command_executedrecords thatentire searchran but not whether it worked.What
New
cli_search_completedPostHog event, emitted once per executed search request onentire searchandentire checkpoint search(same command; both the checkpoint and--codepaths):success(bool),duration_msresult_countandcoverage_incomplete— success only; zero-result rate never counts failures, and a degraded success (failed regions, skipped repos, truncated index) is distinguishable from a genuine zero-result searcherror_class— from typed errors only, never message matching:auth(not logged in, 401/403),cell_skip(auth.ErrNoCellForJurisdiction, client-side),region_unavailable(search.ErrCellUnavailable, gateway without query-serve — kept distinct fromcell_skipper the ticket),repo_unavailable,network,server(5xx),http_other,othercommand,mode(checkpoint/code), and the standard version/os/arch propsPlumbing needed to classify from types:
loginHintErrand the all-cells-skipped path previously replaced typed causes with fresherrors.New— a newhintErrorwrapper keeps the user-facing message byte-identical while leaving the chain matchable witherrors.Is.search.HTTPStatusError(message wording unchanged); code search already returnedapi.HTTPError.Content-free by construction: booleans, enums, counts, durations only — no query text, snippets, or repo names. Gated on the telemetry opt-in setting at the call site and
ENTIRE_TELEMETRY_OPTOUTin the tracker, via the existing detached PostHog child.Scope choices
instrumentSemanticSearcher,searchAllCells), so every entry point emits by construction: one-shot commands, the TUI initial fetch, interactive re-searches, pagination, and the code tab. Each timer wraps only the request — TUI dwell time is never counted. TUI searches fire per submitted query, so volume stays one event per real search request.cli_command_executed.Validation
classifySearchErrortable over real error-chain shapes,hintErrormessage/chain preservation, typedHTTPStatusErrorstatus code.mise run fmt,mise run lint(0 issues),mise run test:ci(unit + integration + canary) all pass.Closes ENT-1938.
🤖 Generated with Claude Code
Note
Low Risk
Search behavior and user-visible errors are unchanged; new telemetry is opt-in, best-effort, and non-blocking with no query or repo data in payloads.
Overview
Adds an opt-in
cli_search_completedPostHog event so product can see whether searches succeed, not just thatentire searchran. Each checkpoint (semantic) and--coderequest emits once right after the network fan-out finishes—before the interactive TUI—soduration_mscovers the API work only.Payloads stay content-free: success, mode, command path, duration,
result_counton success only, anderror_classon failure (never both, so failures aren’t counted as zero-result searches). Emission respects telemetry settings and the existing detached analytics path.Failure
error_classvalues come from typed errors, not message parsing. Supporting changes:search.HTTPStatusErrorfor non-OK semantic responses (same user-facing strings), ahintErrorwrapper so login/region hints keeperrors.Ischains, andclassifySemanticCellswrapping the all-skipped-region case with the underlying skip cause forcell_skipvsregion_unavailable.Reviewed by Cursor Bugbot for commit cd05c83. Configure here.