From 664418634721c17730f7419365ddd8ae637a75cc Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 31 Mar 2026 10:50:06 +0000 Subject: [PATCH] feat(skill): add eval framework to measure SKILL.md effectiveness Two-phase eval: sends test prompts to an LLM with SKILL.md as context, then grades the planned commands on efficiency criteria (no pre-auth, no org lookup, correct fields, minimal calls, trusts auto-detection). - 8 test cases covering the failure modes from issue #598 - Deterministic checks (string matching) + LLM judge (coherence) - Uses Anthropic API (claude-sonnet-4-6, claude-opus-4-6) via repo secret - CI job runs on skill-related file changes, fails below 75% threshold - Fork PRs: blocked until maintainer adds eval-skill label, eval runs via pull_request_target, results posted as commit status - Label removed on synchronize (new push forces re-review) - Uses SENTRY_RELEASE_BOT app token to re-trigger main CI after fork eval --- .github/workflows/ci.yml | 62 +++++- .github/workflows/eval-skill-fork.yml | 97 +++++++++ .gitignore | 1 + AGENTS.md | 94 ++++---- docs/src/content/docs/agent-guidance.md | 31 +-- docs/src/content/docs/commands/dashboard.md | 10 +- package.json | 1 + plugins/sentry-cli/skills/sentry-cli/SKILL.md | 31 +-- .../sentry-cli/references/dashboards.md | 10 +- script/eval-skill.ts | 137 ++++++++++++ src/commands/dashboard/resolve.ts | 47 +--- src/commands/dashboard/widget/add.ts | 11 +- src/commands/dashboard/widget/edit.ts | 17 +- src/commands/issue/explain.ts | 20 +- src/commands/issue/plan.ts | 65 +++--- src/commands/issue/utils.ts | 45 +--- src/lib/arg-parsing.ts | 2 +- src/lib/bspatch.ts | 54 +---- src/lib/db/pagination.ts | 10 +- src/lib/delta-upgrade.ts | 2 - src/lib/formatters/human.ts | 48 ++-- src/lib/polling.ts | 25 +-- src/lib/telemetry.ts | 75 +------ src/types/dashboard.ts | 157 +------------ test/commands/dashboard/widget/add.test.ts | 23 -- test/commands/dashboard/widget/edit.test.ts | 12 +- test/lib/formatters/human.test.ts | 206 ------------------ test/lib/seer-telemetry.test.ts | 56 ----- test/skill-eval/cases.json | 179 +++++++++++++++ test/skill-eval/helpers/judge.ts | 199 +++++++++++++++++ test/skill-eval/helpers/llm-client.ts | 64 ++++++ test/skill-eval/helpers/planner.ts | 90 ++++++++ test/skill-eval/helpers/report.ts | 73 +++++++ test/skill-eval/helpers/types.ts | 73 +++++++ test/types/dashboard.test.ts | 185 +--------------- 35 files changed, 1150 insertions(+), 1062 deletions(-) create mode 100644 .github/workflows/eval-skill-fork.yml create mode 100644 script/eval-skill.ts delete mode 100644 test/lib/seer-telemetry.test.ts create mode 100644 test/skill-eval/cases.json create mode 100644 test/skill-eval/helpers/judge.ts create mode 100644 test/skill-eval/helpers/llm-client.ts create mode 100644 test/skill-eval/helpers/planner.ts create mode 100644 test/skill-eval/helpers/report.ts create mode 100644 test/skill-eval/helpers/types.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c37524928..7ba2dc5691 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,8 @@ jobs: - 'docs/**' - 'plugins/**' - 'script/generate-skill.ts' + - 'script/eval-skill.ts' + - 'test/skill-eval/**' code: - 'src/**' - 'test/**' @@ -133,6 +135,58 @@ jobs: echo "::error::Generated files are out of date. Run 'bun run generate:skill' and 'bun run generate:command-docs' locally and commit the result." exit 1 + eval-skill: + name: Eval SKILL.md + needs: [changes] + if: needs.changes.outputs.skill == 'true' + runs-on: ubuntu-latest + steps: + # For fork PRs: check if eval has already passed via commit status + - name: Detect fork + id: detect-fork + run: | + if [[ "${{ github.event_name }}" == "pull_request" && "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]]; then + echo "is_fork=true" >> "$GITHUB_OUTPUT" + fi + - name: Check fork eval status + if: steps.detect-fork.outputs.is_fork == 'true' + env: + GH_TOKEN: ${{ github.token }} + run: | + SHA="${{ github.event.pull_request.head.sha }}" + STATUS=$(gh api "repos/${{ github.repository }}/commits/$SHA/statuses" \ + --jq '[.[] | select(.context == "eval-skill/fork")] | first | .state // "none"') + if [[ "$STATUS" != "success" ]]; then + echo "::error::Fork PR modifies skill files but eval has not passed for commit $SHA." + echo "::error::A maintainer must review the code and add the 'eval-skill' label." + exit 1 + fi + echo "Fork eval passed for $SHA" + # For internal PRs: run the eval directly + - uses: actions/checkout@v6 + if: steps.detect-fork.outputs.is_fork != 'true' + - uses: oven-sh/setup-bun@v2 + if: steps.detect-fork.outputs.is_fork != 'true' + - uses: actions/cache@v5 + if: steps.detect-fork.outputs.is_fork != 'true' + id: cache + with: + path: node_modules + key: node-modules-${{ hashFiles('bun.lock', 'patches/**') }} + - if: steps.detect-fork.outputs.is_fork != 'true' && steps.cache.outputs.cache-hit != 'true' + run: bun install --frozen-lockfile + - name: Eval SKILL.md + if: steps.detect-fork.outputs.is_fork != 'true' + run: bun run eval:skill + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + - name: Upload eval results + if: always() && steps.detect-fork.outputs.is_fork != 'true' + uses: actions/upload-artifact@v7 + with: + name: skill-eval-results + path: test/skill-eval/results.json + lint: name: Lint & Typecheck needs: [changes] @@ -493,7 +547,7 @@ jobs: ci-status: name: CI Status if: always() - needs: [changes, check-skill, build-binary, build-npm, build-docs, test-e2e, publish-nightly] + needs: [changes, check-skill, eval-skill, build-binary, build-npm, build-docs, test-e2e, publish-nightly] runs-on: ubuntu-latest permissions: {} steps: @@ -501,7 +555,7 @@ jobs: run: | # Check for explicit failures or cancellations in all jobs # publish-nightly is skipped on PRs (if: github.ref == 'refs/heads/main') — that's expected - results="${{ needs.check-skill.result }} ${{ needs.build-binary.result }} ${{ needs.build-npm.result }} ${{ needs.build-docs.result }} ${{ needs.test-e2e.result }} ${{ needs.publish-nightly.result }}" + results="${{ needs.check-skill.result }} ${{ needs.eval-skill.result }} ${{ needs.build-binary.result }} ${{ needs.build-npm.result }} ${{ needs.build-docs.result }} ${{ needs.test-e2e.result }} ${{ needs.publish-nightly.result }}" for result in $results; do if [[ "$result" == "failure" || "$result" == "cancelled" ]]; then echo "::error::CI failed" @@ -519,5 +573,9 @@ jobs: echo "::error::CI failed - upstream job failed causing check-skill to be skipped" exit 1 fi + if [[ "${{ needs.changes.outputs.skill }}" == "true" && "${{ needs.eval-skill.result }}" == "skipped" ]]; then + echo "::error::CI failed - upstream job failed causing eval-skill to be skipped" + exit 1 + fi echo "CI passed" diff --git a/.github/workflows/eval-skill-fork.yml b/.github/workflows/eval-skill-fork.yml new file mode 100644 index 0000000000..5380bf24b8 --- /dev/null +++ b/.github/workflows/eval-skill-fork.yml @@ -0,0 +1,97 @@ +name: Eval SKILL.md (Fork PRs) + +on: + pull_request_target: + types: [labeled, synchronize] + +permissions: + contents: read + statuses: write + pull-requests: write + +jobs: + remove-labels-on-sync: + name: Reset eval labels + if: github.event.action == 'synchronize' + runs-on: ubuntu-latest + steps: + - name: Remove eval labels + env: + GH_TOKEN: ${{ github.token }} + run: | + PR=${{ github.event.number }} + REPO=${{ github.repository }} + gh api "repos/$REPO/issues/$PR/labels/eval-skill" -X DELETE 2>/dev/null || true + gh api "repos/$REPO/issues/$PR/labels/eval-skill-passed" -X DELETE 2>/dev/null || true + + eval: + name: Run skill eval + if: >- + github.event.action == 'labeled' + && github.event.label.name == 'eval-skill' + && github.event.pull_request.head.repo.fork == true + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - uses: oven-sh/setup-bun@v2 + + - uses: actions/cache@v5 + id: cache + with: + path: node_modules + key: node-modules-${{ hashFiles('bun.lock', 'patches/**') }} + - if: steps.cache.outputs.cache-hit != 'true' + run: bun install --frozen-lockfile + + - name: Eval SKILL.md + id: eval + run: bun run eval:skill + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + continue-on-error: true + + - name: Post commit status + env: + GH_TOKEN: ${{ github.token }} + run: | + SHA="${{ github.event.pull_request.head.sha }}" + if [[ "${{ steps.eval.outcome }}" == "success" ]]; then + STATE="success" + DESC="Skill eval passed" + else + STATE="failure" + DESC="Skill eval failed" + fi + gh api "repos/${{ github.repository }}/statuses/$SHA" \ + -f state="$STATE" \ + -f context="eval-skill/fork" \ + -f description="$DESC" + + - name: Remove eval-skill label + if: always() + env: + GH_TOKEN: ${{ github.token }} + run: | + gh api "repos/${{ github.repository }}/issues/${{ github.event.number }}/labels/eval-skill" \ + -X DELETE 2>/dev/null || true + + # Use the SENTRY_RELEASE_BOT app token to add the label — app tokens + # can trigger workflow runs, unlike GITHUB_TOKEN (recursion protection). + - name: Get app token + id: token + if: steps.eval.outcome == 'success' + uses: actions/create-github-app-token@v3 + with: + app-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }} + private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }} + + - name: Add eval-skill-passed label (triggers main CI re-run) + if: steps.eval.outcome == 'success' + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + run: | + gh api "repos/${{ github.repository }}/issues/${{ github.event.number }}/labels" \ + --input - <<< '{"labels":["eval-skill-passed"]}' diff --git a/.gitignore b/.gitignore index 795533ef55..2a8bd491e3 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ coverage-isolated # test artifacts *.junit.xml +test/skill-eval/results.json # logs logs diff --git a/AGENTS.md b/AGENTS.md index 6c7e7e9990..30d54a3606 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -893,65 +893,81 @@ mock.module("./some-module", () => ({ ### Architecture - -* **AsyncIterable streaming for SDK blocked by four structural concerns**: AsyncIterable streaming for SDK implemented via AsyncChannel push/pull pattern. \`src/lib/async-channel.ts\` provides a dual-queue channel: producer calls \`push()\`/\`close()\`/\`error()\`, consumer iterates via \`for await...of\`. \`break\` triggers \`onReturn\` callback for cleanup. \`executeWithStream()\` in \`sdk-invoke.ts\` runs the command in background, pipes \`captureObject\` calls to the channel, and returns the channel immediately. Streaming detection: \`hasStreamingFlag()\` checks for \`--refresh\`/\`--follow\`/\`-f\`. \`buildInvoker\` accepts \`meta.streaming\` flag; \`buildRunner\` auto-detects from args. Abort wiring: \`AbortController\` created per stream, signal placed on fake \`process.abortSignal\`, \`channel.onReturn\` calls \`controller.abort()\`. Both \`log/list.ts\` and \`dashboard/view.ts\` check \`this.process?.abortSignal\` alongside SIGINT. Codegen generates callable interface overloads for streaming commands. + +* **api-client.ts split into domain modules under src/lib/api/**: The original monolithic \`src/lib/api-client.ts\` (1,977 lines) was split into 12 focused domain modules under \`src/lib/api/\`: infrastructure.ts (shared helpers, types, raw requests), organizations.ts, projects.ts, teams.ts, repositories.ts, issues.ts, events.ts, traces.ts, logs.ts, seer.ts, trials.ts, users.ts. The original \`api-client.ts\` was converted to a ~100-line barrel re-export file preserving all existing import paths. The \`biome.jsonc\` override for \`noBarrelFile\` already includes \`api-client.ts\`. When adding new API functions, place them in the appropriate domain module under \`src/lib/api/\`, not in the barrel file. - -* **Bundle uses esbuild with bun:sqlite polyfill plugin for Node.js compatibility**: \`script/bundle.ts\` uses esbuild to produce \`dist/index.cjs\` from \`src/index.ts\`. A \`bunSqlitePlugin\` replaces \`bun:sqlite\` imports with a polyfill. Build defines \`SENTRY\_CLI\_VERSION\` and \`SENTRY\_CLIENT\_ID\_BUILD\`, externalizes \`node:\*\` builtins. \`sentrySourcemapPlugin\` handles debug ID injection and sourcemap upload. After the main build, writes: (1) \`dist/bin.cjs\` — CLI wrapper with shebang/Node version check/warning suppression, (2) \`dist/index.d.cts\` — type declarations read from pre-built \`src/sdk.generated.d.cts\`. Both \`sdk.generated.\*\` files are gitignored and regenerated via \`generate:sdk\` script chained before \`bundle\` in \`package.json\`. Debug IDs solve sourcemap deduplication between npm bundle and bun compile builds. + +* **Bun compiled binary sourcemap options and size impact**: Binary build (\`script/build.ts\`) uses two steps: (1) \`Bun.build()\` produces \`dist-bin/bin.js\` + \`.map\` with \`sourcemap: "linked"\` and minification. (2) \`Bun.build()\` with \`compile: true\` produces native binary — no sourcemap embedded. Bun's compiled binaries use \`/$bunfs/root/bin.js\` as the virtual path in stack traces. Sourcemap upload must use \`--url-prefix '/$bunfs/root/'\` so Sentry can match frames. The upload runs \`sentry-cli sourcemaps inject dist-bin/\` first (adds debug IDs), then uploads both JS and map. Bun's compile step strips comments (including \`//# debugId=\`), but debug ID matching still works via the injected runtime snippet + URL prefix matching. Size: +0.04 MB gzipped vs +2.30 MB for inline sourcemaps. Without \`SENTRY\_AUTH\_TOKEN\`, upload is skipped gracefully. - -* **CLI logic extracted from bin.ts into cli.ts for shared entry points**: \`src/cli.ts\` contains the full CLI runner extracted from \`bin.ts\`: \`runCompletion()\` (shell completion fast path), \`runCli()\` (full CLI with middleware — auto-auth, seer trial, unknown command telemetry), and \`startCli()\` (top-level dispatch). All functions are exported, no top-level execution. \`src/bin.ts\` is a thin ~30-line wrapper for bun compile that registers EPIPE/EIO stream error handlers and calls \`startCli()\`. The npm bin wrapper (\`dist/bin.cjs\`) is a ~300-byte generated script that \`require('./index.cjs').\_cli()\`. Both entry points share the same CLI logic via \`cli.ts\`. + +* **CLI telemetry DSN is public write-only — safe to embed in install script**: The CLI's Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key already baked into every binary. Safe to hardcode in install scripts. Opt-out: \`SENTRY\_CLI\_NO\_TELEMETRY=1\`. - -* **Library API: variadic sentry() function with last-arg options detection**: \`createSentrySDK(options?)\` in \`src/index.ts\` is the sole public API. Returns a typed SDK object with methods for every CLI command plus \`run()\` escape hatch. \`SentryOptions\` in \`src/lib/sdk-types.ts\`: \`token?\`, \`text?\` (run-only), \`cwd?\`, \`url?\` (self-hosted base URL → \`SENTRY\_HOST\`), \`org?\` (default org → \`SENTRY\_ORG\`), \`project?\` (default project → \`SENTRY\_PROJECT\`). Env isolation via \`buildIsolatedEnv(options)\` helper in \`sdk-invoke.ts\` — shared by both \`buildInvoker\` and \`buildRunner\`, maps each option to its env var. Zero-copy \`captureObject\` return, \`OutputError\` → data recovery. Default JSON output via \`SENTRY\_OUTPUT\_FORMAT=json\`. Non-zero exit throws \`SentryError\` with \`.exitCode\` and \`.stderr\`. + +* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow. - -* **Library mode telemetry strips all global-polluting Sentry integrations**: When \`initSentry(enabled, { libraryMode: true })\` is called, the Sentry SDK initializes without integrations that pollute the host process. \`LIBRARY\_EXCLUDED\_INTEGRATIONS\` extends the base set with: \`OnUncaughtException\`, \`OnUnhandledRejection\`, \`ProcessSession\` (process listeners), \`Http\`/\`NodeFetch\` (trace header injection), \`FunctionToString\` (wraps \`Function.prototype.toString\`), \`ChildProcess\`/\`NodeContext\`. Also disables \`enableLogs\` and \`sendClientReports\` (both use timers/\`beforeExit\`), and skips \`process.on('beforeExit')\` handler registration. Keeps pure integrations: \`eventFiltersIntegration\`, \`linkedErrorsIntegration\`. Library entry manually calls \`client.flush(3000)\` after command completion (both success and error paths via \`flushTelemetry()\` helper). Only unavoidable global: \`globalThis.\_\_SENTRY\_\_\[SDK\_VERSION]\`. + +* **Nightly delta upgrade buildNightlyPatchGraph fetches ALL patch tags — O(N) HTTP calls**: Delta upgrade in \`src/lib/delta-upgrade.ts\` supports stable (GitHub Releases) and nightly (GHCR) channels. \`filterAndSortChainTags\` filters \`patch-\*\` tags by version range using \`Bun.semver.order()\`. GHCR uses \`fetchWithRetry\` (10s timeout + 1 retry; blobs 30s) with optional \`signal?: AbortSignal\` combined via \`AbortSignal.any()\`. \`isExternalAbort(error, signal)\` skips retries for external aborts — critical for background prefetch. Patches cached to \`~/.sentry/patch-cache/\` (file-based, 7-day TTY). \`loadCachedChain\` stitches patches for multi-hop offline upgrades. - -* **Typed SDK uses direct Command.loader() invocation bypassing Stricli dispatch**: \`createSentrySDK(options?)\` in \`src/index.ts\` builds a typed namespace API (\`sdk.org.list()\`, \`sdk.issue.view()\`) generated by \`script/generate-sdk.ts\`. At runtime, \`src/lib/sdk-invoke.ts\` resolves commands via Stricli route tree, caches \`Command\` objects, and calls \`command.loader()\` directly — bypassing string dispatch and flag parsing. The standalone variadic \`sentry()\` function has been removed. Typed SDK methods are the primary path, with \`sdk.run()\` as an escape hatch for arbitrary CLI strings (interactive commands like \`auth login\`, raw \`api\` passthrough). The codegen auto-discovers ALL commands from the route tree with zero config, using CLI route names as-is (\`org.list\`, \`dashboard.widget.add\`). Return types are derived from \`\_\_jsonSchema\` when present, otherwise \`unknown\`. Positional patterns are derived from introspection placeholder strings. Hidden routes (plural aliases) are skipped. + +* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses \`node:sqlite\`. A runtime version guard in the esbuild banner catches this early. When writing esbuild banner strings in TS template literals, double-escape: \`\\\\\\\n\` in TS → \`\\\n\` in output → newline at runtime. Single \`\\\n\` produces a literal newline inside a JS string, causing SyntaxError. -### Decision + +* **Numeric issue ID resolution returns org:undefined despite API success**: Numeric issue ID resolution in \`resolveNumericIssue()\`: (1) try DSN/env/config for org, (2) if found use \`getIssueInOrg(org, id)\` with region routing, (3) else fall back to unscoped \`getIssue(id)\`, (4) extract org from \`issue.permalink\` via \`parseSentryUrl\` as final fallback. \`parseSentryUrl\` handles path-based (\`/organizations/{org}/...\`) and subdomain-style URLs. \`matchSubdomainOrg()\` filters region subdomains by requiring slug length > 2. Self-hosted uses path-based only. + + +* **Seer trial prompt uses middleware layering in bin.ts error handling chain**: Error recovery middlewares in \`bin.ts\` are layered: \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (for \`no\_budget\`/\`not\_enabled\`) caught by inner wrapper; auth errors bubble to outer. Auth retry goes through full chain. Trial API: \`GET /api/0/customers/{org}/\` → \`productTrials\[]\` (prefer \`seerUsers\`, fallback \`seerAutofix\`). Start: \`PUT /api/0/customers/{org}/product-trial/\`. SaaS-only; self-hosted 404s gracefully. \`ai\_disabled\` excluded. \`startSeerTrial\` accepts \`category\` from trial object — don't hardcode. - -* **Agent skill files embedded at build time, not fetched from network**: \`agent-skills.ts\` imports skill content from \`src/generated/skill-content.ts\` (a build-time generated module) and writes files directly to \`~/.claude/skills/sentry-cli/\`. No network fetching, no GitHub URLs, no fallback URLs, no \`REFERENCE\_FILES\` array. \`installAgentSkills(homeDir)\` takes only \`homeDir\` — no version parameter needed since content is baked into the binary. The generated module is ~47KB of inlined markdown strings, comparable to \`sdk.generated.ts\`. Works identically for both bun compile (native binary) and esbuild npm bundle since it's a standard TS module. Previously skill files were fetched from \`raw.githubusercontent.com\` with \`cli.sentry.dev\` fallback — a roundtrip for content already available at build time. + +* **SQLite DB functions are synchronous — async signatures are historical artifacts**: All \`src/lib/db/\` functions do synchronous SQLite operations (both \`bun:sqlite\` and the \`node:sqlite\` polyfill's \`DatabaseSync\` are sync). Many functions still have \`async\` signatures — this is a historical artifact from PR #89 which migrated config storage from JSON files (using async \`Bun.file().text()\` / \`Bun.write()\`) to SQLite. The function signatures were preserved to minimize diff size and never cleaned up. These can safely be converted to synchronous. Exceptions that ARE legitimately async: \`clearAuth()\` (cache dir cleanup), \`getCachedDetection()\`/\`getCachedProjectRoot()\`/\`setCachedProjectRoot()\` (stat for mtime), \`refreshToken()\`/\`performTokenRefresh()\` (HTTP calls). - -* **OutputError propagates via throw instead of process.exit()**: The \`process.exit()\` call in \`command.ts\` (OutputError handler) is replaced with \`throw err\` to support library mode. \`OutputError\` is re-thrown through Stricli via \`exceptionWhileRunningCommand\` in \`app.ts\` (added before the \`AuthError\` check), so Stricli never writes an error message for it. In CLI mode (\`cli.ts\`), OutputError is caught and \`process.exitCode\` is set silently without writing to stderr (data was already rendered). In library mode (\`index.ts\`), the catch block checks if \`capturedResult\` has data (the OutputError's payload was rendered to stdout via \`captureObject\` before the throw) and returns it instead of throwing \`SentryError\`. This eliminates the only \`process.exit()\` outside of \`bin.ts\`. +### Decision - -* **SDK codegen moving to auto-generate all commands from route tree**: \`script/generate-sdk.ts\` walks the Stricli route tree via \`discoverCommands()\`, skipping hidden routes. For each command: extracts flags, derives positional params from placeholder strings, checks \`\_\_jsonSchema\` for typed return types. Naming uses CLI route path as-is: \`\["org", "list"]\` → \`sdk.org.list()\`. Generates TWO gitignored files: (1) \`src/sdk.generated.ts\` — runtime, (2) \`src/sdk.generated.d.cts\` — npm type declarations. \`generate:sdk\` is chained before \`typecheck\`, \`dev\`, \`build\`, \`build:all\`, \`bundle\`. \`INTERNAL\_FLAGS\` set excludes \`json\`, \`fields\`, \`refresh\`, \`follow\` from generated parameter types — streaming flags are library-incompatible. CI check \`bun run check:skill\` validates SKILL.md stays in sync. + +* **Raw markdown output for non-interactive terminals, rendered for TTY**: Markdown-first output pipeline: custom renderer in \`src/lib/formatters/markdown.ts\` walks \`marked\` tokens to produce ANSI-styled output. Commands build CommonMark using helpers (\`mdKvTable()\`, \`mdRow()\`, \`colorTag()\`, \`escapeMarkdownCell()\`, \`safeCodeSpan()\`) and pass through \`renderMarkdown()\`. \`isPlainOutput()\` precedence: \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`FORCE\_COLOR\` > \`!isTTY\`. \`--json\` always outputs JSON. Colors defined in \`COLORS\` object in \`colors.ts\`. Tests run non-TTY so assertions match raw CommonMark; use \`stripAnsi()\` helper for rendered-mode assertions. + + +* **whoami should be separate from auth status command**: The \`sentry auth whoami\` command should be a dedicated command separate from \`sentry auth status\`. They serve different purposes: \`status\` shows everything about auth state (token, expiry, defaults, org verification), while \`whoami\` just shows user identity (name, email, username, ID) by fetching live from \`/auth/\` endpoint. \`sentry whoami\` should be a top-level alias (like \`sentry issues\` → \`sentry issue list\`). \`whoami\` should support \`--json\` for machine consumption and be lightweight — no credential verification, no defaults listing. ### Gotcha - -* **Test mocks lack process property — use optional chaining on this.process**: Command \`func()\` methods access \`this: SentryContext\` which has \`this.process\`. But test mocks created via \`createMockContext()\` only provide \`stdout\`/\`stderr\`/\`cwd\` — no \`process\` property. Accessing \`this.process.abortSignal\` crashes with \`undefined is not an object\`. Fix: always use optional chaining \`(this.process as T)?.abortSignal\` or check \`this.process\` exists first. This applies to any new property added to the process-like object in \`sdk-invoke.ts\` that commands read via \`this.process\`. + +* **@sentry/api SDK passes Request object to custom fetch — headers lost on Node.js**: @sentry/api SDK calls \`\_fetch(request)\` with no init object. In \`authenticatedFetch\`, \`init\` is undefined so \`prepareHeaders\` creates empty headers — on Node.js this strips Content-Type (HTTP 415). Fix: fall back to \`input.headers\` when \`init\` is undefined. Use \`unwrapPaginatedResult\` (not \`unwrapResult\`) to access the Response's Link header for pagination. \`per\_page\` is not in SDK types; cast query to pass it at runtime. -### Pattern + +* **Bun binary build requires SENTRY\_CLIENT\_ID env var**: The build script (\`script/bundle.ts\`) requires \`SENTRY\_CLIENT\_ID\` environment variable and exits with code 1 if missing. When building locally, use \`bun run --env-file=.env.local build\` or set the env var explicitly. The binary build (\`bun run build\`) also needs it. Without it you get: \`Error: SENTRY\_CLIENT\_ID environment variable is required.\` - -* **buildIsolatedEnv helper centralizes SDK env setup**: \`buildIsolatedEnv(options?)\` in \`src/lib/sdk-invoke.ts\` maps \`SentryOptions\` fields to env vars (\`token\` → \`SENTRY\_AUTH\_TOKEN\`, \`url\` → \`SENTRY\_HOST\`, etc.) plus \`SENTRY\_OUTPUT\_FORMAT=json\` (unless \`text: true\`). The core dedup is \`executeWithCapture\()\` which centralizes the env isolation → capture context → telemetry → error wrapping → output parsing pipeline. Both \`buildInvoker\` (typed methods) and \`buildRunner\` (\`run()\` escape hatch) are thin ~15-line wrappers providing only the executor callback. \`STREAMING\_FLAGS\` set (\`--refresh\`, \`--follow\`, \`-f\`) is checked in \`buildRunner\` before execution — throws \`SentryError\` immediately since streaming output is unsuitable for library mode. Same flags are in \`INTERNAL\_FLAGS\` in codegen so typed SDK methods can't trigger streaming. + +* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly builds publish to GHCR with versioned tags like \`nightly-0.14.0-dev.1772661724\`, not GitHub Releases or npm. \`fetchManifest()\` throws \`UpgradeError("network\_error")\` for both network failures and non-200 — callers must check message for HTTP 404/403. Craft with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if only target is \`github\`. - -* **Command docs use GENERATED:END marker for hybrid auto/manual content**: \`generate-command-docs.ts\` produces one \`.md\` per visible route in \`docs/src/content/docs/commands/\`. Each page splits at \`\\`: above is auto-generated (flags, args, descriptions from Stricli introspection), below is hand-written (examples, guides). Regeneration preserves custom content below the marker. \`check-command-docs.ts\` only compares the auto-generated portion. \`GLOBAL\_FLAG\_NAMES\` (\`json\`, \`fields\`) are excluded from per-command docs. The \`SKIP\_ROUTES\` set filters hidden plural aliases. Pages must contain bash code blocks for the skill generator to extract examples — \`check-skill.ts\` validates this via \`\*\*Examples:\*\*\` section check. + +* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`. - -* **SDK codegen callable interface pattern for streaming overloads**: In \`script/generate-sdk.ts\`, streaming-capable commands (those with flags in \`STREAMING\_FLAGS\` set) use a callable interface pattern instead of a simple method signature. This produces TypeScript overloaded signatures: \`(params?: T): Promise\\` for non-streaming and \`(params: T & { follow: string }): AsyncIterable\\` for streaming. At runtime, \`generateStreamingMethodBody()\` emits code that checks if any streaming flag is present in params, then passes \`{ streaming: true }\` meta to the invoker which branches to \`executeWithStream\` vs \`executeWithCapture\`. The \`STREAMING\_FLAGS\` set (\`refresh\`, \`follow\`) is separate from \`INTERNAL\_FLAGS\` — streaming flags ARE included in generated params but excluded from \`INTERNAL\_FLAGS\`. + +* **Multi-region fan-out: distinguish all-403 from empty orgs with hasSuccessfulRegion flag**: In \`listOrganizationsUncached\` (\`src/lib/api/organizations.ts\`), \`Promise.allSettled\` collects multi-region results. Don't use \`flatResults.length === 0\` to detect all-regions-failed — a region returning 200 OK with zero orgs pushes nothing into \`flatResults\`. Track a \`hasSuccessfulRegion\` boolean on any \`"fulfilled"\` settlement. Only re-throw 403 \`ApiError\` when \`!hasSuccessfulRegion && lastScopeError\`. + + +* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling it twice replaces the first mock. Use a single unified fetch mock dispatching by URL pattern. (2) \`mock.module()\` pollutes the module registry for ALL subsequent test files. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. This also causes \`delta-upgrade.test.ts\` to fail when run alongside \`test/isolated/delta-upgrade.test.ts\` — the isolated test's \`mock.module()\` replaces \`CLI\_VERSION\` for all subsequent files. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`. + + +* **useTestConfigDir without isolateProjectRoot causes DSN scanning of repo tree**: \`useTestConfigDir()\` creates temp dirs under \`.test-tmp/\` in the repo tree. Without \`{ isolateProjectRoot: true }\`, \`findProjectRoot\` walks up and finds the repo's \`.git\`, causing DSN detection to scan real source code and trigger network calls against test mocks (timeouts). Always pass \`isolateProjectRoot: true\` when tests exercise \`resolveOrg\`, \`detectDsn\`, or \`findProjectRoot\`. + +### Pattern - -* **SENTRY\_OUTPUT\_FORMAT env var enables JSON mode from env instead of --json flag**: In \`src/lib/command.ts\`, the \`wrappedFunc\` checks \`this.env?.SENTRY\_OUTPUT\_FORMAT === "json"\` to force JSON output mode without passing \`--json\` on the command line. This is how the library entry point (\`src/index.ts\`) gets JSON by default — it sets this env var in the isolated env. The check runs after \`cleanRawFlags\` and only when the command has an \`output\` config (supports JSON). Commands without JSON support (help, version) are unaffected. ~5-line addition to \`command.ts\`. + +* **findProjectsByPattern as fuzzy fallback for exact slug misses**: When \`findProjectsBySlug\` returns empty (no exact match), use \`findProjectsByPattern\` as a fallback to suggest similar projects. \`findProjectsByPattern\` does bidirectional word-boundary matching (\`matchesWordBoundary\`) against all projects in all orgs — the same logic used for directory name inference. In the \`project-search\` handler, call it after the exact miss, format matches as \`\/\\` suggestions in the \`ResolutionError\`. This avoids a dead-end error for typos like 'patagonai' when 'patagon-ai' exists. Note: \`findProjectsByPattern\` makes additional API calls (lists all projects per org), so only call it on the failure path. - -* **Skill files generation pipeline and staleness checks**: \`generate-skill.ts\` walks the Stricli route tree to produce \`SKILL.md\` + per-group \`references/\*.md\` + \`index.json\` under \`plugins/sentry-cli/skills/sentry-cli/\`, AND generates \`src/generated/skill-content.ts\` — a TypeScript module that inlines all skill file contents as a \`ReadonlyMap\\`. This module is imported by \`agent-skills.ts\` to write files to disk without any network fetching. The generator must write a stub \`skill-content.ts\` before dynamically importing \`src/app.ts\` (chicken-and-egg: app.ts transitively imports agent-skills.ts which imports skill-content.ts). Uses \`await import()\` not static \`import\` for the route tree. \`generate:skill\` is chained before \`build\`, \`dev\`, \`typecheck\`, and \`test\` in package.json, like \`generate:sdk\`. \`check-skill.ts\` validates both skill files AND \`skill-content.ts\` for staleness. + +* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves. - -* **Target argument 4-mode parsing convention (project-search-first)**: \`parseOrgProjectArg()\` in \`src/lib/arg-parsing.ts\` returns a 4-mode discriminated union: \`auto-detect\` (empty), \`explicit\` (\`org/project\`), \`org-all\` (\`org/\` trailing slash), \`project-search\` (bare slug). Bare slugs are ALWAYS \`project-search\` first. The "is this an org?" check is secondary: list commands with \`orgSlugMatchBehavior\` pre-check cached orgs (\`redirect\` or \`error\` mode), and \`handleProjectSearch()\` has a safety net checking orgs after project search fails. Non-list commands (init, view) treat bare slugs purely as project search with no org pre-check. For \`init\`, unmatched bare slugs become new project names. Key files: \`src/lib/arg-parsing.ts\` (parsing), \`src/lib/org-list.ts\` (dispatch + org pre-check), \`src/lib/resolve-target.ts\` (resolution cascade). + +* **PR workflow: wait for Seer and Cursor BugBot before resolving**: CI includes Seer Code Review and Cursor Bugbot as advisory checks (~2-3 min, only on ready-for-review PRs). Workflow: push → wait for all CI (including npm build) → check inline review comments from Seer/BugBot → fix valid findings → repeat. Bugbot sometimes catches real logic bugs, not just style — always review before merging. Use \`gh pr checks \ --watch\` to monitor. Fetch comments via \`gh api repos/OWNER/REPO/pulls/NUM/comments\`. - -* **Writer type is the minimal output interface for streams and mocks**: The \`Writer\` type in \`src/types/index.ts\` is \`{ write(data: string): void; captureObject?: (obj: unknown) => void }\`. The optional \`captureObject\` property replaces the previous duck-typing pattern (\`hasCaptureObject()\` with \`typeof\` check and \`Record\\` cast). In library mode, the writer sets \`captureObject\` to capture the fully-transformed JSON object directly without serialization. In CLI mode, \`process.stdout\` lacks this property so it's \`undefined\` → falsy, and \`emitJsonObject()\` falls through to \`JSON.stringify\`. The check is now a simple truthiness test: \`if (stdout.captureObject)\`. Since \`captureObject\` is part of the \`Writer\` type, \`sdk-invoke.ts\` no longer needs \`Writer & { captureObject?: ... }\` intersection types — plain \`Writer\` suffices. + +* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: Schema v12 replaced \`pagination\_cursors.cursor TEXT\` with \`cursor\_stack TEXT\` (JSON array) + \`page\_index INTEGER\`. Stack-based API in \`src/lib/db/pagination.ts\`: \`resolveCursor(flag, key, contextKey)\` maps keywords (next/prev/previous/first/last) to \`{cursor, direction}\`. \`advancePaginationState(key, contextKey, direction, nextCursor)\` pushes/pops the stack — back-then-forward truncates stale entries. \`hasPreviousPage(key, contextKey)\` checks \`page\_index > 0\`. \`clearPaginationState(key)\` removes state. \`parseCursorFlag\` in \`list-command.ts\` accepts next/prev/previous/first/last keywords. \`paginationHint()\` in \`org-list.ts\` builds bidirectional hints (\`-c prev | -c next\`). JSON envelope includes \`hasPrev\` boolean. All 7 list commands (trace, span, issue, project, team, repo, dashboard) use this stack API. \`resolveCursor()\` must be called inside \`org-all\` override closures. -### Preference + +* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` — no-op without active transaction. User-visible fallbacks use \`log.warn()\` not \`log.debug()\`. Several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\` (trace/list, trace/view, log/view, api.ts, help.ts). - -* **Library features require README and docs site updates**: When adding new features like the library API, documentation must be updated in both places: the root \`README.md\` (library usage section between Configuration and Development, before the \`---\` divider) and the docs website at \`docs/src/content/docs/\`. The docs site uses Astro + Starlight with sidebar defined in \`docs/astro.config.mjs\`. New pages outside \`commands/\` must be manually added to the sidebar config. \`library-usage.md\` was added to the "Getting Started" sidebar section after "Configuration". Note: \`features.md\` and \`agent-guidance.md\` exist but are NOT in the sidebar. + +* **Testing Stricli command func() bodies via spyOn mocking**: To unit-test a Stricli command's \`func()\` body: (1) \`const func = await cmd.loader()\`, (2) \`func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. (3) \`spyOn\` namespace imports to mock dependencies (e.g., \`spyOn(apiClient, 'getLogs')\`). The \`loader()\` return type union causes \`.call()\` LSP errors — these are false positives that pass \`tsc --noEmit\`. When API functions are renamed (e.g., \`getLog\` → \`getLogs\`), update both spy target name AND mock return shape (single → array). Slug normalization (\`normalizeSlug\`) replaces underscores with dashes but does NOT lowercase — test assertions must match original casing (e.g., \`'CAM-82X'\` not \`'cam-82x'\`). diff --git a/docs/src/content/docs/agent-guidance.md b/docs/src/content/docs/agent-guidance.md index d0bc85c339..0dc3578d8e 100644 --- a/docs/src/content/docs/agent-guidance.md +++ b/docs/src/content/docs/agent-guidance.md @@ -128,23 +128,7 @@ Display types with default sizes: Use **common** types for general dashboards. Use **specialized** only when specifically requested. Avoid **internal** types unless the user explicitly asks. -**Dataset selection:** - -Use the default `spans` dataset for most widgets — it covers spans, and transactions when filtered with `is_transaction:true`. - -| Dataset | Use for | Notes | -|---|---|---| -| `spans` (default) | Spans, transactions (with `is_transaction:true`) | Preferred for most widgets | -| `issue` | Issue-based widgets (issue list, status counts) | Use `table` display | -| `error-events` | Error events only | Replaces deprecated `discover` for errors | -| `metrics` | Custom metrics | | -| `logs` | Log-based widgets | | - -**Widget constraints:** - -- `--group-by` **requires** `--limit` — the API will reject the widget without it -- Table widgets cap at **10 rows** max — `--limit` values above 10 are clamped -- `--sort -count` — the leading `-` can be misinterpreted as a flag. Use `--sort="-count"` (with `=`) to avoid flag alias conflicts +Available datasets: `spans` (default, covers most use cases), `discover`, `issue`, `error-events`, `transaction-like`, `metrics`, `logs`, `tracemetrics`, `preprod-app-size`. Run `sentry dashboard widget --help` for the full list including aggregate functions. @@ -163,15 +147,7 @@ sentry dashboard widget add "Latency Over Time" --display line --que # Full-width table (6 = 6) sentry dashboard widget add "Top Endpoints" --display table \ --query count --query p95:span.duration \ - --group-by transaction --sort="-count" --limit 10 - -# Line chart grouped by tag (--group-by requires --limit) -sentry dashboard widget add "Errors by Browser" \ - --display line --query count --group-by browser.name --limit 10 - -# Querying transactions (not just spans) — add is_transaction:true -sentry dashboard widget add "Transaction Count" \ - --display big_number --query count --where "is_transaction:true" + --group-by transaction --sort -count --limit 10 ``` ## Common Mistakes @@ -184,6 +160,3 @@ sentry dashboard widget add "Transaction Count" \ - **Not using `--web`**: View commands support `-w`/`--web` to open the resource in the browser — useful for sharing links. - **Fetching API schemas instead of using the CLI**: Prefer `sentry schema` to browse the API and `sentry api` to make requests — the CLI handles authentication and endpoint resolution, so there's rarely a need to download OpenAPI specs separately. - **Using `sentry api` when CLI commands suffice**: `sentry issue list --json` already includes `shortId`, `title`, `priority`, `level`, `status`, `permalink`, and other fields at the top level. Some fields like `count`, `userCount`, `firstSeen`, and `lastSeen` may be null depending on the issue. Use `--fields` to select specific fields and `--help` to see all available fields. Only fall back to `sentry api` for data the CLI doesn't expose. -- **Using deprecated dashboard datasets**: `--dataset discover` and `--dataset transaction-like` are rejected — the CLI validates against accepted datasets. Use the default `spans` dataset. To query transactions, add `--where "is_transaction:true"`. -- **Forgetting `--limit` with `--group-by`**: Dashboard widgets with `--group-by` require `--limit` — the API will reject the widget without it. Table widgets cap at 10 rows. -- **`--sort` with leading dash**: `--sort -count` is misinterpreted as the `-c` flag alias. Use `--sort="-count"` (with `=`) instead. diff --git a/docs/src/content/docs/commands/dashboard.md b/docs/src/content/docs/commands/dashboard.md index f96b12346b..e3cdd8bdad 100644 --- a/docs/src/content/docs/commands/dashboard.md +++ b/docs/src/content/docs/commands/dashboard.md @@ -70,12 +70,12 @@ Add a widget to a dashboard | Option | Description | |--------|-------------| | `-d, --display ` | Display type (big_number, line, area, bar, table, stacked_area, top_n, text, categorical_bar, details, wheel, rage_and_dead_clicks, server_tree, agents_traces_table) | -| `--dataset ` | Widget dataset: issue, metrics, error-events, spans, logs, tracemetrics, preprod-app-size (default: spans) | +| `--dataset ` | Widget dataset (default: spans) | | `-q, --query ...` | Aggregate expression (e.g. count, p95:span.duration) | | `-w, --where ` | Search conditions filter (e.g. is:unresolved) | -| `-g, --group-by ...` | Group-by column (repeatable). Requires --limit | -| `-s, --sort ` | Order by (prefix - for desc). Use --sort="-count" (with =) to avoid flag alias conflicts | -| `-n, --limit ` | Result limit. Required when using --group-by. Table widgets cap at 10 rows | +| `-g, --group-by ...` | Group-by column (repeatable) | +| `-s, --sort ` | Order by (prefix - for desc, e.g. -count) | +| `-n, --limit ` | Result limit | | `--x ` | Grid column position (0-based, 0–5) | | `--y ` | Grid row position (0-based) | | `--width ` | Widget width in grid columns (1–6) | @@ -99,7 +99,7 @@ Edit a widget in a dashboard | `-t, --title ` | Widget title to match | | `--new-title <new-title>` | New widget title | | `-d, --display <display>` | Display type (big_number, line, area, bar, table, stacked_area, top_n, text, categorical_bar, details, wheel, rage_and_dead_clicks, server_tree, agents_traces_table) | -| `--dataset <dataset>` | Widget dataset: issue, metrics, error-events, spans, logs, tracemetrics, preprod-app-size (default: spans) | +| `--dataset <dataset>` | Widget dataset (default: spans) | | `-q, --query <query>...` | Aggregate expression (e.g. count, p95:span.duration) | | `-w, --where <where>` | Search conditions filter (e.g. is:unresolved) | | `-g, --group-by <group-by>...` | Group-by column (repeatable) | diff --git a/package.json b/package.json index 82fbb88585..b57277cf53 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "generate:skill": "bun run script/generate-skill.ts", "generate:schema": "bun run script/generate-api-schema.ts", "generate:command-docs": "bun run script/generate-command-docs.ts", + "eval:skill": "bun run script/eval-skill.ts", "check:skill": "bun run script/check-skill.ts", "check:command-docs": "bun run script/check-command-docs.ts", "check:deps": "bun run script/check-no-deps.ts", diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 3c937ce717..8a66eedfdb 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -138,23 +138,7 @@ Display types with default sizes: Use **common** types for general dashboards. Use **specialized** only when specifically requested. Avoid **internal** types unless the user explicitly asks. -**Dataset selection:** - -Use the default `spans` dataset for most widgets — it covers spans, and transactions when filtered with `is_transaction:true`. - -| Dataset | Use for | Notes | -|---|---|---| -| `spans` (default) | Spans, transactions (with `is_transaction:true`) | Preferred for most widgets | -| `issue` | Issue-based widgets (issue list, status counts) | Use `table` display | -| `error-events` | Error events only | Replaces deprecated `discover` for errors | -| `metrics` | Custom metrics | | -| `logs` | Log-based widgets | | - -**Widget constraints:** - -- `--group-by` **requires** `--limit` — the API will reject the widget without it -- Table widgets cap at **10 rows** max — `--limit` values above 10 are clamped -- `--sort -count` — the leading `-` can be misinterpreted as a flag. Use `--sort="-count"` (with `=`) to avoid flag alias conflicts +Available datasets: `spans` (default, covers most use cases), `discover`, `issue`, `error-events`, `transaction-like`, `metrics`, `logs`, `tracemetrics`, `preprod-app-size`. Run `sentry dashboard widget --help` for the full list including aggregate functions. @@ -173,15 +157,7 @@ sentry dashboard widget add <dashboard> "Latency Over Time" --display line --que # Full-width table (6 = 6) sentry dashboard widget add <dashboard> "Top Endpoints" --display table \ --query count --query p95:span.duration \ - --group-by transaction --sort="-count" --limit 10 - -# Line chart grouped by tag (--group-by requires --limit) -sentry dashboard widget add <dashboard> "Errors by Browser" \ - --display line --query count --group-by browser.name --limit 10 - -# Querying transactions (not just spans) — add is_transaction:true -sentry dashboard widget add <dashboard> "Transaction Count" \ - --display big_number --query count --where "is_transaction:true" + --group-by transaction --sort -count --limit 10 ``` ### Common Mistakes @@ -194,9 +170,6 @@ sentry dashboard widget add <dashboard> "Transaction Count" \ - **Not using `--web`**: View commands support `-w`/`--web` to open the resource in the browser — useful for sharing links. - **Fetching API schemas instead of using the CLI**: Prefer `sentry schema` to browse the API and `sentry api` to make requests — the CLI handles authentication and endpoint resolution, so there's rarely a need to download OpenAPI specs separately. - **Using `sentry api` when CLI commands suffice**: `sentry issue list --json` already includes `shortId`, `title`, `priority`, `level`, `status`, `permalink`, and other fields at the top level. Some fields like `count`, `userCount`, `firstSeen`, and `lastSeen` may be null depending on the issue. Use `--fields` to select specific fields and `--help` to see all available fields. Only fall back to `sentry api` for data the CLI doesn't expose. -- **Using deprecated dashboard datasets**: `--dataset discover` and `--dataset transaction-like` are rejected — the CLI validates against accepted datasets. Use the default `spans` dataset. To query transactions, add `--where "is_transaction:true"`. -- **Forgetting `--limit` with `--group-by`**: Dashboard widgets with `--group-by` require `--limit` — the API will reject the widget without it. Table widgets cap at 10 rows. -- **`--sort` with leading dash**: `--sort -count` is misinterpreted as the `-c` flag alias. Use `--sort="-count"` (with `=`) instead. ## Prerequisites diff --git a/plugins/sentry-cli/skills/sentry-cli/references/dashboards.md b/plugins/sentry-cli/skills/sentry-cli/references/dashboards.md index 167197fd72..f9ed7d1254 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/dashboards.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/dashboards.md @@ -76,12 +76,12 @@ Add a widget to a dashboard **Flags:** - `-d, --display <value> - Display type (big_number, line, area, bar, table, stacked_area, top_n, text, categorical_bar, details, wheel, rage_and_dead_clicks, server_tree, agents_traces_table)` -- `--dataset <value> - Widget dataset: issue, metrics, error-events, spans, logs, tracemetrics, preprod-app-size (default: spans)` +- `--dataset <value> - Widget dataset (default: spans)` - `-q, --query <value>... - Aggregate expression (e.g. count, p95:span.duration)` - `-w, --where <value> - Search conditions filter (e.g. is:unresolved)` -- `-g, --group-by <value>... - Group-by column (repeatable). Requires --limit` -- `-s, --sort <value> - Order by (prefix - for desc). Use --sort="-count" (with =) to avoid flag alias conflicts` -- `-n, --limit <value> - Result limit. Required when using --group-by. Table widgets cap at 10 rows` +- `-g, --group-by <value>... - Group-by column (repeatable)` +- `-s, --sort <value> - Order by (prefix - for desc, e.g. -count)` +- `-n, --limit <value> - Result limit` - `--x <value> - Grid column position (0-based, 0–5)` - `--y <value> - Grid row position (0-based)` - `--width <value> - Widget width in grid columns (1–6)` @@ -121,7 +121,7 @@ Edit a widget in a dashboard - `-t, --title <value> - Widget title to match` - `--new-title <value> - New widget title` - `-d, --display <value> - Display type (big_number, line, area, bar, table, stacked_area, top_n, text, categorical_bar, details, wheel, rage_and_dead_clicks, server_tree, agents_traces_table)` -- `--dataset <value> - Widget dataset: issue, metrics, error-events, spans, logs, tracemetrics, preprod-app-size (default: spans)` +- `--dataset <value> - Widget dataset (default: spans)` - `-q, --query <value>... - Aggregate expression (e.g. count, p95:span.duration)` - `-w, --where <value> - Search conditions filter (e.g. is:unresolved)` - `-g, --group-by <value>... - Group-by column (repeatable)` diff --git a/script/eval-skill.ts b/script/eval-skill.ts new file mode 100644 index 0000000000..47fcce9063 --- /dev/null +++ b/script/eval-skill.ts @@ -0,0 +1,137 @@ +#!/usr/bin/env bun +/** + * Evaluate SKILL.md effectiveness by testing LLM command planning. + * + * Sends test prompts to agent models (Opus 4.6 + Sonnet 4.6) with SKILL.md + * as context, then grades the planned commands on efficiency criteria. + * + * Requires ANTHROPIC_API_KEY env var for Anthropic API access. + * In CI, the key is stored in the "skill-eval" environment (protected). + * + * Usage: + * bun run eval:skill + * EVAL_AGENT_MODELS=claude-sonnet-4-6-20250627 bun run eval:skill + * + * Environment variables: + * ANTHROPIC_API_KEY - Anthropic API key (required) + * EVAL_AGENT_MODELS - Comma-separated model IDs (default: sonnet-4-6, opus-4-6) + * EVAL_JUDGE_MODEL - Judge model ID (default: haiku-4-5) + * EVAL_THRESHOLD - Minimum pass rate 0-1 (default: 0.75) + */ + +import cases from "../test/skill-eval/cases.json"; +import { judgePlan } from "../test/skill-eval/helpers/judge.js"; +import { createClient } from "../test/skill-eval/helpers/llm-client.js"; +import { generatePlan } from "../test/skill-eval/helpers/planner.js"; +import { + getExitCode, + printReport, + writeJsonReport, +} from "../test/skill-eval/helpers/report.js"; +import type { + CaseResult, + EvalReport, + ModelResult, + TestCase, +} from "../test/skill-eval/helpers/types.js"; + +const SKILL_PATH = "plugins/sentry-cli/skills/sentry-cli/SKILL.md"; +const RESULTS_PATH = "test/skill-eval/results.json"; +const VERSION_RE = /^version:\s*(.+)$/m; + +/** + * Default pass threshold — set from baseline run (2026-03-30). + * Baseline: openai/gpt-4.1 scored 100% (8/8 cases). + * Set to 75% to allow for LLM non-determinism while catching regressions. + */ +const DEFAULT_THRESHOLD = 0.75; + +/** Run all eval cases against a single model */ +async function evalModel( + client: Awaited<ReturnType<typeof createClient>>, + model: string, + skillContent: string, + testCases: TestCase[] +): Promise<ModelResult> { + console.log(`\nEvaluating: ${model}`); + console.log("─".repeat(40)); + + const results: CaseResult[] = []; + + for (const testCase of testCases) { + process.stdout.write(` ${testCase.id}... `); + + const plan = await generatePlan( + client, + model, + skillContent, + testCase.prompt + ); + const result = await judgePlan(client, testCase, plan); + results.push(result); + + const icon = result.passed ? "✓" : "✗"; + console.log(`${icon} (${(result.score * 100).toFixed(0)}%)`); + } + + const totalPassed = results.filter((r) => r.passed).length; + const score = results.length > 0 ? totalPassed / results.length : 0; + + return { + model, + cases: results, + totalPassed, + totalCases: results.length, + score, + }; +} + +async function main(): Promise<void> { + const apiKey = process.env.ANTHROPIC_API_KEY; + if (!apiKey) { + console.error("Error: ANTHROPIC_API_KEY is required for the skill eval."); + console.error("Set it via: export ANTHROPIC_API_KEY=<your-key>"); + process.exit(1); + } + + const client = await createClient(apiKey); + const skillContent = await Bun.file(SKILL_PATH).text(); + const testCases = cases as unknown as TestCase[]; + const threshold = process.env.EVAL_THRESHOLD + ? Number.parseFloat(process.env.EVAL_THRESHOLD) + : DEFAULT_THRESHOLD; + + console.log( + `Skill eval: ${testCases.length} cases × ${client.agentModels.length} models` + ); + console.log(`Agent models: ${client.agentModels.join(", ")}`); + console.log(`Judge model: ${client.judgeModel}`); + console.log(`Threshold: ${(threshold * 100).toFixed(0)}%`); + + // Extract skill version from YAML frontmatter + const versionMatch = skillContent.match(VERSION_RE); + const skillVersion = versionMatch?.[1]?.trim() ?? "unknown"; + + const models: ModelResult[] = []; + for (const model of client.agentModels) { + const result = await evalModel(client, model, skillContent, testCases); + models.push(result); + } + + const report: EvalReport = { + timestamp: new Date().toISOString(), + skillVersion, + threshold, + models, + }; + + printReport(report); + await writeJsonReport(report, RESULTS_PATH); + + process.exit(getExitCode(report)); +} + +main().catch((err) => { + console.error("Fatal error:", err); + process.exit(1); +}); diff --git a/src/commands/dashboard/resolve.ts b/src/commands/dashboard/resolve.ts index 82db623b01..1ad13f2294 100644 --- a/src/commands/dashboard/resolve.ts +++ b/src/commands/dashboard/resolve.ts @@ -477,48 +477,13 @@ export function enrichDashboardError( throw error; } -/** Migration hints for deprecated dataset names */ -const DEPRECATED_DATASET_HINTS: Record<string, string> = { - discover: - 'The "discover" dataset is deprecated. Use "error-events" for errors or "spans" for transactions/spans.', - "transaction-like": - 'The "transaction-like" dataset is deprecated. Use "spans" with --where "is_transaction:true" instead.', -}; - -/** - * Reject deprecated or unknown dataset values with actionable hints. - * Extracted to keep validateWidgetEnums under the complexity limit. - */ -function rejectInvalidDataset(dataset: string): void { - const deprecationHint = DEPRECATED_DATASET_HINTS[dataset]; - if (deprecationHint) { - throw new ValidationError(deprecationHint, "dataset"); - } - if (!WIDGET_TYPES.includes(dataset as (typeof WIDGET_TYPES)[number])) { - throw new ValidationError( - `Invalid --dataset value "${dataset}".\nValid datasets: ${WIDGET_TYPES.join(", ")}`, - "dataset" - ); - } -} - /** * Validate --display and --dataset flag values against known enums. * - * Rejects deprecated datasets (`discover`, `transaction-like`) with - * actionable migration hints before making any API calls. - * * @param display - Display type flag value * @param dataset - Dataset flag value - * @param options.skipDeprecatedCheck - Skip the deprecated dataset check. - * Used by `widget edit` when the dataset comes from the existing widget - * rather than from explicit `--dataset` input. */ -export function validateWidgetEnums( - display?: string, - dataset?: string, - options?: { skipDeprecatedCheck?: boolean } -): void { +export function validateWidgetEnums(display?: string, dataset?: string): void { if ( display && !DISPLAY_TYPES.includes(display as (typeof DISPLAY_TYPES)[number]) @@ -528,8 +493,14 @@ export function validateWidgetEnums( "display" ); } - if (dataset && !options?.skipDeprecatedCheck) { - rejectInvalidDataset(dataset); + if ( + dataset && + !WIDGET_TYPES.includes(dataset as (typeof WIDGET_TYPES)[number]) + ) { + throw new ValidationError( + `Invalid --dataset value "${dataset}".\nValid datasets: ${WIDGET_TYPES.join(", ")}`, + "dataset" + ); } if (display && dataset) { // Untracked display types (text, wheel, rage_and_dead_clicks, agents_traces_table) diff --git a/src/commands/dashboard/widget/add.ts b/src/commands/dashboard/widget/add.ts index 0efdbf8f6e..b4163dd9fa 100644 --- a/src/commands/dashboard/widget/add.ts +++ b/src/commands/dashboard/widget/add.ts @@ -19,7 +19,6 @@ import { FALLBACK_LAYOUT, prepareDashboardForUpdate, validateWidgetLayout, - WIDGET_TYPES, type WidgetLayoutFlags, } from "../../../types/dashboard.js"; import { @@ -120,7 +119,7 @@ export const addCommand = buildCommand({ dataset: { kind: "parsed", parse: String, - brief: `Widget dataset: ${WIDGET_TYPES.join(", ")} (default: spans)`, + brief: "Widget dataset (default: spans)", optional: true, }, query: { @@ -139,22 +138,20 @@ export const addCommand = buildCommand({ "group-by": { kind: "parsed", parse: String, - brief: "Group-by column (repeatable). Requires --limit", + brief: "Group-by column (repeatable)", variadic: true, optional: true, }, sort: { kind: "parsed", parse: String, - brief: - 'Order by (prefix - for desc). Use --sort="-count" (with =) to avoid flag alias conflicts', + brief: "Order by (prefix - for desc, e.g. -count)", optional: true, }, limit: { kind: "parsed", parse: numberParser, - brief: - "Result limit. Required when using --group-by. Table widgets cap at 10 rows", + brief: "Result limit", optional: true, }, x: { diff --git a/src/commands/dashboard/widget/edit.ts b/src/commands/dashboard/widget/edit.ts index 3714f12831..342190791f 100644 --- a/src/commands/dashboard/widget/edit.ts +++ b/src/commands/dashboard/widget/edit.ts @@ -24,7 +24,6 @@ import { prepareWidgetQueries, validateAggregateNames, validateWidgetLayout, - WIDGET_TYPES, type WidgetLayoutFlags, } from "../../../types/dashboard.js"; import { @@ -123,16 +122,12 @@ function buildReplacement( const effectiveDisplay = flags.display ?? existing.displayType; const effectiveDataset = flags.dataset ?? existing.widgetType; - // Validate user-provided --dataset against deprecated types. - // Only check flags.dataset (not effectiveDataset) so editing a widget with - // a deprecated widgetType (e.g., "discover") doesn't trigger the deprecation - // check when the user isn't changing datasets. - // Cross-validate display×dataset using effective values so that --display - // changes that conflict with the existing dataset are still caught. + // Re-validate after merging with existing values. validateWidgetEnums only + // checks the cross-constraint when both args are provided, so it misses + // e.g. `--dataset preprod-app-size` on a widget that's already `table`. + // validateWidgetEnums itself skips untracked display types (text, wheel, etc.). if (flags.display || flags.dataset) { - validateWidgetEnums(effectiveDisplay, effectiveDataset, { - skipDeprecatedCheck: !flags.dataset, - }); + validateWidgetEnums(effectiveDisplay, effectiveDataset); } const raw: Record<string, unknown> = { @@ -212,7 +207,7 @@ export const editCommand = buildCommand({ dataset: { kind: "parsed", parse: String, - brief: `Widget dataset: ${WIDGET_TYPES.join(", ")} (default: spans)`, + brief: "Widget dataset (default: spans)", optional: true, }, query: { diff --git a/src/commands/issue/explain.ts b/src/commands/issue/explain.ts index 03732e6c38..c76102b65d 100644 --- a/src/commands/issue/explain.ts +++ b/src/commands/issue/explain.ts @@ -6,18 +6,20 @@ import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; +import { ApiError } from "../../lib/errors.js"; import { CommandOutput } from "../../lib/formatters/output.js"; -import { formatRootCauseList } from "../../lib/formatters/seer.js"; +import { + formatRootCauseList, + handleSeerApiError, +} from "../../lib/formatters/seer.js"; import { applyFreshFlag, FRESH_ALIASES, FRESH_FLAG, } from "../../lib/list-command.js"; -import { recordSeerOutcome } from "../../lib/telemetry.js"; import { extractRootCauses } from "../../types/seer.js"; import { ensureRootCauseAnalysis, - handleSeerCommandError, issueIdPositional, resolveOrgAndIssueId, } from "./utils.js"; @@ -74,8 +76,8 @@ export const explainCommand = buildCommand({ applyFreshFlag(flags); const { cwd } = this; + // Declare org outside try block so it's accessible in catch for error messages let resolvedOrg: string | undefined; - let recorded = false; try { // Resolve org and issue ID @@ -97,8 +99,6 @@ export const explainCommand = buildCommand({ // Extract root causes from steps const causes = extractRootCauses(state); if (causes.length === 0) { - recorded = true; - recordSeerOutcome("no_solution"); throw new Error( "Analysis completed but no root causes found. " + "The issue may not have enough context for root cause analysis." @@ -106,11 +106,13 @@ export const explainCommand = buildCommand({ } yield new CommandOutput(causes); - recorded = true; - recordSeerOutcome("success"); return { hint: `To create a plan, run: sentry issue plan ${issueArg}` }; } catch (error) { - handleSeerCommandError(error, recorded, resolvedOrg); + // Handle API errors with friendly messages + if (error instanceof ApiError) { + throw handleSeerApiError(error.status, error.detail, resolvedOrg); + } + throw error; } }, }); diff --git a/src/commands/issue/plan.ts b/src/commands/issue/plan.ts index bf13c6a37b..8d133d7e40 100644 --- a/src/commands/issue/plan.ts +++ b/src/commands/issue/plan.ts @@ -8,16 +8,18 @@ import type { SentryContext } from "../../context.js"; import { triggerSolutionPlanning } from "../../lib/api-client.js"; import { buildCommand, numberParser } from "../../lib/command.js"; -import { ValidationError } from "../../lib/errors.js"; +import { ApiError, ValidationError } from "../../lib/errors.js"; import { CommandOutput } from "../../lib/formatters/output.js"; -import { formatSolution } from "../../lib/formatters/seer.js"; +import { + formatSolution, + handleSeerApiError, +} from "../../lib/formatters/seer.js"; import { applyFreshFlag, FRESH_ALIASES, FRESH_FLAG, } from "../../lib/list-command.js"; import { logger } from "../../lib/logger.js"; -import { recordSeerOutcome } from "../../lib/telemetry.js"; import { type AutofixState, extractRootCauses, @@ -27,7 +29,6 @@ import { } from "../../types/seer.js"; import { ensureRootCauseAnalysis, - handleSeerCommandError, issueIdPositional, pollAutofixState, resolveOrgAndIssueId, @@ -41,6 +42,23 @@ type PlanFlags = { readonly fields?: string[]; }; +/** + * Validate that the autofix state has root causes identified. + * + * @param state - Current autofix state (already ensured to exist) + * @returns Array of root causes + * @throws {ValidationError} If no root causes found + */ +function validateRootCauses(state: AutofixState): RootCause[] { + const causes = extractRootCauses(state); + if (causes.length === 0) { + throw new ValidationError( + "No root causes identified. Cannot create a plan without a root cause." + ); + } + return causes; +} + /** * Validate and resolve the cause selection for solution planning. * @@ -177,8 +195,8 @@ export const planCommand = buildCommand({ applyFreshFlag(flags); const { cwd } = this; + // Declare org outside try block so it's accessible in catch for error messages let resolvedOrg: string | undefined; - let recorded = false; try { // Resolve org and issue ID @@ -196,16 +214,8 @@ export const planCommand = buildCommand({ json: flags.json, }); - // Validate we have root causes — record no_solution before throwing - // so the dashboard tracks this as a missing-data case, not a generic error - const causes = extractRootCauses(state); - if (causes.length === 0) { - recorded = true; - recordSeerOutcome("no_solution"); - throw new ValidationError( - "No root causes identified. Cannot create a plan without a root cause." - ); - } + // Validate we have root causes + const causes = validateRootCauses(state); // Validate cause selection const causeId = validateCauseSelection(causes, flags.cause, issueArg); @@ -215,10 +225,7 @@ export const planCommand = buildCommand({ if (!flags.force) { const existingSolution = extractSolution(state); if (existingSolution) { - yield new CommandOutput(buildPlanData(state)); - recorded = true; - recordSeerOutcome("success"); - return; + return yield new CommandOutput(buildPlanData(state)); } } @@ -233,7 +240,7 @@ export const planCommand = buildCommand({ await triggerSolutionPlanning(org, numericId, state.run_id); - // Poll until plan is created + // Poll until PR is created const finalState = await pollAutofixState({ orgSlug: org, issueId: numericId, @@ -245,28 +252,24 @@ export const planCommand = buildCommand({ ` Or retry: sentry issue plan ${issueArg}`, }); - // Handle terminal error states + // Handle errors if (finalState.status === "ERROR") { - recorded = true; - recordSeerOutcome("error"); throw new Error( "Plan creation failed. Check the Sentry web UI for details." ); } if (finalState.status === "CANCELLED") { - recorded = true; - recordSeerOutcome("error"); throw new Error("Plan creation was cancelled."); } - const planData = buildPlanData(finalState); - yield new CommandOutput(planData); - recorded = true; - recordSeerOutcome(planData.solution ? "success" : "no_solution"); - return; + return yield new CommandOutput(buildPlanData(finalState)); } catch (error) { - handleSeerCommandError(error, recorded, resolvedOrg); + // Handle API errors with friendly messages + if (error instanceof ApiError) { + throw handleSeerApiError(error.status, error.detail, resolvedOrg); + } + throw error; } }, }); diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index d1a6e20c53..5cb0df8be0 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -29,10 +29,7 @@ import { ResolutionError, withAuthGuard, } from "../../lib/errors.js"; -import { - getProgressMessage, - handleSeerApiError, -} from "../../lib/formatters/seer.js"; +import { getProgressMessage } from "../../lib/formatters/seer.js"; import { expandToFullShortId, isShortSuffix } from "../../lib/issue-id.js"; import { logger } from "../../lib/logger.js"; import { poll } from "../../lib/polling.js"; @@ -44,11 +41,7 @@ import { } from "../../lib/resolve-target.js"; import { parseSentryUrl } from "../../lib/sentry-url-parser.js"; import { buildIssueUrl } from "../../lib/sentry-urls.js"; -import { - classifySeerError, - recordSeerOutcome, - setOrgProjectContext, -} from "../../lib/telemetry.js"; +import { setOrgProjectContext } from "../../lib/telemetry.js"; import { isAllDigits } from "../../lib/utils.js"; import type { SentryIssue } from "../../types/index.js"; import { type AutofixState, isTerminalStatus } from "../../types/seer.js"; @@ -814,37 +807,3 @@ export async function pollAutofixState( initialMessage: "Waiting for analysis to start...", }); } - -/** - * Handle errors in Seer commands with outcome recording. - * - * Records the Seer outcome if not already recorded, maps API errors to - * Seer-specific errors, and re-throws. Shared between explain and plan - * commands to keep outcome classification consistent. - * - * @param error - The caught error - * @param recorded - Whether outcome was already recorded before the error - * @param resolvedOrg - Org slug for Seer error messages - * @returns never — always throws - */ -export function handleSeerCommandError( - error: unknown, - recorded: boolean, - resolvedOrg: string | undefined -): never { - if (!recorded) { - if (error instanceof ApiError) { - const mapped = handleSeerApiError( - error.status, - error.detail, - resolvedOrg - ); - recordSeerOutcome(classifySeerError(mapped)); - throw mapped; - } - recordSeerOutcome(classifySeerError(error)); - } else if (error instanceof ApiError) { - throw handleSeerApiError(error.status, error.detail, resolvedOrg); - } - throw error; -} diff --git a/src/lib/arg-parsing.ts b/src/lib/arg-parsing.ts index 1185318311..4105f90f50 100644 --- a/src/lib/arg-parsing.ts +++ b/src/lib/arg-parsing.ts @@ -315,7 +315,7 @@ export function parseSpanDepth(input: string): number { return 0; } const n = Number(input); - if (Number.isNaN(n) || n < 0) { + if (Number.isNaN(n)) { return DEFAULT_SPAN_DEPTH; } return n; diff --git a/src/lib/bspatch.ts b/src/lib/bspatch.ts index 2366159738..25c98ec27b 100644 --- a/src/lib/bspatch.ts +++ b/src/lib/bspatch.ts @@ -38,12 +38,6 @@ import { join } from "node:path"; /** TRDIFF10 header magic bytes */ const TRDIFF10_MAGIC = "TRDIFF10"; -/** Yield to the event loop every ~40ms so the spinner stays responsive (25fps, under the 50ms spinner interval) */ -const FRAME_BUDGET_MS = 40; - -/** Only call performance.now() every 4096 bytes to amortize the cost */ -const YIELD_CHECK_INTERVAL = 4096; - /** Header size in bytes (magic + 3 × i64) */ const HEADER_SIZE = 32; @@ -283,41 +277,6 @@ async function loadOldBinary(oldPath: string): Promise<OldFileHandle> { } } -/** Arguments for {@link addDiffBytesWithYield} */ -type DiffYieldArgs = { - output: Uint8Array; - oldFile: Uint8Array; - diffChunk: Uint8Array; - oldpos: number; - lastYield: number; -}; - -/** - * Add diff bytes to old file bytes with wrapping unsigned addition, yielding - * to the event loop periodically so the spinner stays responsive. - * - * Checks `performance.now()` every {@link YIELD_CHECK_INTERVAL} bytes and - * yields when the elapsed time exceeds {@link FRAME_BUDGET_MS}. Returns the - * updated `lastYield` timestamp for the caller to carry across calls. - */ -async function addDiffBytesWithYield(args: DiffYieldArgs): Promise<number> { - const { output, oldFile, diffChunk, oldpos } = args; - let yieldTs = args.lastYield; - - for (let i = 0; i < output.length; i++) { - output[i] = ((oldFile[oldpos + i] ?? 0) + (diffChunk[i] ?? 0)) % 256; - - if (i > 0 && i % YIELD_CHECK_INTERVAL === 0) { - const now = performance.now(); - if (now - yieldTs >= FRAME_BUDGET_MS) { - await Bun.sleep(0); - yieldTs = performance.now(); - } - } - } - return yieldTs; -} - /** * Apply a TRDIFF10 binary patch with streaming I/O for minimal memory usage. * @@ -366,7 +325,6 @@ export async function applyPatch( let oldpos = 0; let newpos = 0; - let lastYield = performance.now(); try { // Process control entries: each is 3 × i64 = 24 bytes @@ -384,13 +342,11 @@ export async function applyPatch( const diffChunk = await diffReader.read(readDiffBy); const outputChunk = new Uint8Array(readDiffBy); - lastYield = await addDiffBytesWithYield({ - output: outputChunk, - oldFile, - diffChunk, - oldpos, - lastYield, - }); + for (let i = 0; i < readDiffBy; i++) { + // Wrapping unsigned byte addition, matching zig-bsdiff's @addWithOverflow + outputChunk[i] = + ((oldFile[oldpos + i] ?? 0) + (diffChunk[i] ?? 0)) % 256; + } writer.write(outputChunk); hasher.update(outputChunk); diff --git a/src/lib/db/pagination.ts b/src/lib/db/pagination.ts index c51029fd76..c702918d96 100644 --- a/src/lib/db/pagination.ts +++ b/src/lib/db/pagination.ts @@ -84,15 +84,7 @@ export function getPaginationState( return; } - let stack: string[]; - try { - stack = JSON.parse(row.cursor_stack) as string[]; - } catch { - db.query( - "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?" - ).run(commandKey, contextKey); - return; - } + const stack = JSON.parse(row.cursor_stack) as string[]; return { stack, index: row.page_index }; } diff --git a/src/lib/delta-upgrade.ts b/src/lib/delta-upgrade.ts index 3256133106..5ef561554d 100644 --- a/src/lib/delta-upgrade.ts +++ b/src/lib/delta-upgrade.ts @@ -39,7 +39,6 @@ import { } from "./ghcr.js"; import { logger } from "./logger.js"; import { loadCachedChain, savePatchesToCache } from "./patch-cache.js"; -import { setProgressMessage } from "./polling.js"; import { withTracing, withTracingSpan } from "./telemetry.js"; /** Scoped logger for delta upgrade operations */ @@ -1135,7 +1134,6 @@ async function applyPatchesSequentially( if (!patch) { throw new Error(`Missing patch at index ${i}`); } - setProgressMessage(`Applying patch ${i + 1}/${chain.patches.length}...`); const isLast = i === chain.patches.length - 1; const intermediate = i % 2 === 0 ? intermediateA : intermediateB; const outputPath = isLast ? destPath : intermediate; diff --git a/src/lib/formatters/human.ts b/src/lib/formatters/human.ts index 128377b9d6..540a9a533b 100644 --- a/src/lib/formatters/human.ts +++ b/src/lib/formatters/human.ts @@ -2141,48 +2141,38 @@ function formatChangelog(data: UpgradeResult): string { } const { changelog } = data; - const mdLines: string[] = ["### What's new ✨", ""]; + const lines: string[] = ["", "### What's new", ""]; for (const section of changelog.sections) { - // Skip sections whose markdown is empty after whitespace trimming - if (!section.markdown.trim()) { - continue; - } - mdLines.push(CATEGORY_HEADINGS[section.category]); - mdLines.push(section.markdown); - } - - // Nothing to show after filtering empty sections - if (mdLines.length <= 2) { - return ""; + lines.push(CATEGORY_HEADINGS[section.category]); + lines.push(section.markdown); } if (changelog.truncated) { const more = changelog.originalCount - changelog.totalItems; - mdLines.push( + lines.push( `<muted>...and ${more} more changes — https://github.com/getsentry/cli/releases</muted>` ); } - // Render through the markdown pipeline - const rendered = renderMarkdown(mdLines.join("\n")); - - // Indent all lines by 2 spaces so the changelog visually nests under - // the upgrade status header, then clamp to terminal height. - const indented = rendered - .split("\n") - .map((line) => (line ? ` ${line}` : line)); - + // Render through the markdown pipeline, then clamp to terminal height + const rendered = renderMarkdown(lines.join("\n")); + const renderedLines = rendered.split("\n"); const maxLines = getMaxChangelogLines(); - if (indented.length > maxLines) { - const clamped = indented.slice(0, maxLines); - const moreText = - " ...and more — https://github.com/getsentry/cli/releases"; - clamped.push(isPlainOutput() ? moreText : muted(moreText)); - return `\n${clamped.join("\n")}\n`; + + if (renderedLines.length <= maxLines) { + return rendered; } - return `\n${indented.join("\n")}\n`; + // Truncate and add a "more" indicator + const clamped = renderedLines.slice(0, maxLines); + const remaining = changelog.originalCount - changelog.totalItems; + const moreText = + remaining > 0 + ? "...and more — https://github.com/getsentry/cli/releases" + : "...truncated — https://github.com/getsentry/cli/releases"; + clamped.push(isPlainOutput() ? moreText : muted(moreText)); + return clamped.join("\n"); } /** Action descriptions for human-readable output */ diff --git a/src/lib/polling.ts b/src/lib/polling.ts index 539b09e7e7..8857fe79ea 100644 --- a/src/lib/polling.ts +++ b/src/lib/polling.ts @@ -5,7 +5,6 @@ * Used by commands that need to wait for async operations to complete. */ -import { AsyncLocalStorage } from "node:async_hooks"; import { TimeoutError } from "./errors.js"; import { isPlainOutput } from "./formatters/plain-detect.js"; import { @@ -13,26 +12,6 @@ import { truncateProgressMessage, } from "./formatters/seer.js"; -/** - * Async-propagated progress context. - * - * Stores the active spinner's `setMessage` callback so any function in the - * async call chain can update the spinner text without explicit parameter - * threading — the same pattern Sentry SDK / OpenTelemetry use for span - * propagation. When no spinner is active, `getStore()` returns `undefined` - * and {@link setProgressMessage} becomes a no-op. - */ -const progressStorage = new AsyncLocalStorage<(msg: string) => void>(); - -/** - * Update the active spinner message from anywhere in the async call chain. - * No-op when no spinner is active (JSON mode, non-TTY, or outside - * {@link withProgress}). - */ -export function setProgressMessage(msg: string): void { - progressStorage.getStore()?.(msg); -} - /** Default polling interval in milliseconds */ const DEFAULT_POLL_INTERVAL_MS = 1000; @@ -230,9 +209,7 @@ export async function withProgress<T>( const spinner = startSpinner(options.message); try { - return await progressStorage.run(spinner.setMessage, () => - fn(spinner.setMessage) - ); + return await fn(spinner.setMessage); } finally { spinner.stop(); process.stdout.write("\r\x1b[K"); diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index a0f0786d4d..027f59e4da 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -19,7 +19,7 @@ import { } from "./constants.js"; import { isReadonlyError, tryRepairAndRetry } from "./db/schema.js"; import { getEnv } from "./env.js"; -import { ApiError, AuthError, CliError, TimeoutError } from "./errors.js"; +import { ApiError, AuthError } from "./errors.js"; import { attachSentryReporter } from "./logger.js"; import { getSentryBaseUrl, isSentrySaasUrl } from "./sentry-urls.js"; import { getRealUsername } from "./utils.js"; @@ -1110,76 +1110,3 @@ export function withCacheSpan<T>( } ); } - -// Seer Command Telemetry - -/** - * Possible outcomes for Seer AI commands. - * - * Set as the `seer.outcome` tag on the `cli.command` span so Discover - * can slice Seer command executions by result category. - */ -export type SeerOutcome = - | "success" - | "timeout" - | "no_budget" - | "not_enabled" - | "ai_disabled" - | "no_solution" - | "api_error" - | "error"; - -/** - * Record the outcome of a Seer AI command as a span tag. - * - * Sets `seer.outcome` on the current scope so it lands on the - * `cli.command` transaction span. Since tracesSampleRate is 1, - * every execution is captured — no separate metrics needed. - * - * Call exactly once per command execution. - * - * @param outcome - What happened (success, timeout, no_budget, etc.) - */ -export function recordSeerOutcome(outcome: SeerOutcome): void { - Sentry.setTag("seer.outcome", outcome); -} - -/** - * Classify an error into a SeerOutcome. - * - * Maps known error types to their corresponding outcome values: - * - SeerError → matches `.reason` (no_budget, not_enabled, ai_disabled) - * - TimeoutError → "timeout" - * - ApiError → "api_error" - * - Everything else → "error" - * - * @param error - The caught error - * @returns The corresponding SeerOutcome - */ -export function classifySeerError(error: unknown): SeerOutcome { - // SeerError extends CliError and has .reason: SeerErrorReason - if ( - error instanceof CliError && - "reason" in error && - typeof error.reason === "string" - ) { - const reason = error.reason as string; - if ( - reason === "no_budget" || - reason === "not_enabled" || - reason === "ai_disabled" - ) { - return reason; - } - } - - if (error instanceof TimeoutError) { - return "timeout"; - } - - if (error instanceof ApiError) { - return "api_error"; - } - - return "error"; -} diff --git a/src/types/dashboard.ts b/src/types/dashboard.ts index 27b9e992d5..7c12fbf3a1 100644 --- a/src/types/dashboard.ts +++ b/src/types/dashboard.ts @@ -19,43 +19,23 @@ import { logger } from "../lib/logger.js"; // --------------------------------------------------------------------------- /** - * Widget types (dataset selectors) the API still accepts for creation. + * Valid widget types (dataset selectors). * * Source: sentry/src/sentry/models/dashboard_widget.py DashboardWidgetTypes.TYPES */ export const WIDGET_TYPES = [ + "discover", "issue", "metrics", "error-events", + "transaction-like", "spans", "logs", "tracemetrics", "preprod-app-size", ] as const; -/** - * Deprecated widget types rejected by the Sentry Dashboard API. - * - * - `discover` → use `error-events` for errors or `spans` for everything else - * - `transaction-like` → use `spans` with `is_transaction:true` filter - */ -export const DEPRECATED_WIDGET_TYPES = [ - "discover", - "transaction-like", -] as const; - -export type DeprecatedWidgetType = (typeof DEPRECATED_WIDGET_TYPES)[number]; - -/** - * All widget types including deprecated — used for parsing server responses - * where existing dashboards may still reference old types. - */ -export const ALL_WIDGET_TYPES = [ - ...WIDGET_TYPES, - ...DEPRECATED_WIDGET_TYPES, -] as const; - -export type WidgetType = (typeof ALL_WIDGET_TYPES)[number]; +export type WidgetType = (typeof WIDGET_TYPES)[number]; /** Default widgetType — the modern spans dataset covers most use cases */ export const DEFAULT_WIDGET_TYPE: WidgetType = "spans"; @@ -188,17 +168,12 @@ export type DashboardDetail = z.infer<typeof DashboardDetailSchema>; * Defaults widgetType to "spans" when not provided. * * Use DashboardWidgetSchema (permissive) for parsing server responses. - * - * Accepts ALL_WIDGET_TYPES (including deprecated) so that editing existing - * widgets with deprecated types (e.g., "discover") doesn't fail validation. - * CLI-level rejection of deprecated types for new widgets is handled by - * validateWidgetEnums() in resolve.ts. */ export const DashboardWidgetInputSchema = z .object({ title: z.string(), displayType: z.enum(DISPLAY_TYPES), - widgetType: z.enum(ALL_WIDGET_TYPES).default(DEFAULT_WIDGET_TYPE), + widgetType: z.enum(WIDGET_TYPES).default(DEFAULT_WIDGET_TYPE), interval: z.string().optional(), queries: z.array(DashboardWidgetQuerySchema).optional(), layout: DashboardWidgetLayoutSchema.optional(), @@ -503,126 +478,8 @@ function extractFunctionName(aggregate: string): string { return parenIdx > 0 ? aggregate.slice(0, parenIdx) : aggregate; } -/** - * Extract the argument from a parsed aggregate string. - * "count()" → "" - * "p95(span.duration)" → "span.duration" - * "avg(g:custom/foo@b)" → "g:custom/foo@b" - */ -function extractAggregateArg(aggregate: string): string { - const openIdx = aggregate.indexOf("("); - const closeIdx = aggregate.lastIndexOf(")"); - if (openIdx < 0 || closeIdx <= openIdx) { - return ""; - } - return aggregate.slice(openIdx + 1, closeIdx); -} - -// --------------------------------------------------------------------------- -// MRI (Metric Resource Identifier) detection -// -// Port of Sentry's canonical Python parser: -// sentry/src/sentry/snuba/metrics/naming_layer/mri.py -// -// MRI format: <entity>:<namespace>/<name>@<unit> -// entity: "c" (counter), "d" (distribution), "g" (gauge), "s" (set), -// "e" (extracted), or multi-char like "dist" -// namespace: "sessions", "transactions", "spans", "custom", etc. -// name: metric name, e.g. "duration", "measurements.fcp" -// unit: "none", "millisecond", "byte", "byte/second", etc. -// --------------------------------------------------------------------------- - -/** - * Regex matching the MRI schema format. - * - * Uses the same permissive `[^:]+` entity pattern as the canonical Python parser - * so it catches multi-char entity codes like "dist" in addition to the standard - * single-letter codes (c, d, g, s, e). - */ -const MRI_RE = - /^(?<entity>[^:]+):(?<namespace>[^/]+)\/(?<name>[^@]+)@(?<unit>.+)$/; - -/** Parsed components of an MRI string */ -export type ParsedMri = { - /** Entity type code (e.g. "c", "d", "g", "s", "e", or "dist") */ - entity: string; - /** Metric namespace (e.g. "custom", "sessions", "transactions") */ - namespace: string; - /** Metric name (e.g. "duration", "node.runtime.mem.rss") */ - name: string; - /** Metric unit (e.g. "byte", "millisecond", "none") */ - unit: string; -}; - -/** - * Parse an MRI (Metric Resource Identifier) string into its components. - * - * Port of Sentry's `parse_mri` from `sentry/snuba/metrics/naming_layer/mri.py`. - * - * @returns Parsed MRI components, or null if the input doesn't match MRI format - */ -export function parseMri(input: string): ParsedMri | null { - const match = MRI_RE.exec(input); - if (!match?.groups) { - return null; - } - const { entity, namespace, name, unit } = match.groups; - if (!(entity && namespace && name && unit)) { - return null; - } - return { entity, namespace, name, unit }; -} - -/** Maps known MRI entity codes to human-readable tracemetrics type names */ -const MRI_ENTITY_TYPE_NAMES: Record<string, string> = { - c: "counter", - d: "distribution", - g: "gauge", - s: "set", - e: "extracted", -}; - -/** - * Check aggregates for MRI syntax and throw a ValidationError with guidance - * on using `--dataset tracemetrics` with the correct query format. - * - * MRI queries like `avg(g:custom/node.runtime.mem.rss@byte)` pass function-name - * validation (since "avg" is valid) but produce widgets that render as - * "Internal Error" in the Sentry dashboard UI. - * - * @param aggregates - Parsed aggregate strings to check - * @param dataset - Current dataset flag value (for context in error message) - */ -function rejectMriQueries(aggregates: string[], dataset?: string): void { - for (const agg of aggregates) { - const arg = extractAggregateArg(agg); - const mri = parseMri(arg); - if (!mri) { - continue; - } - - const fn = extractFunctionName(agg); - const typeName = MRI_ENTITY_TYPE_NAMES[mri.entity] ?? mri.entity; - const suggestion = `${fn}(value,${mri.name},${typeName},${mri.unit})`; - const datasetNote = - dataset === "tracemetrics" - ? "" - : `\nUse --dataset tracemetrics instead of --dataset ${dataset ?? "spans"}.`; - - throw new ValidationError( - `MRI query syntax is not supported for dashboard widgets: "${agg}".\n\n` + - "Use the tracemetrics query format instead:\n" + - ` --query '${suggestion}'${datasetNote}\n\n` + - "Tracemetrics format: fn(value,<metric_name>,<type>,<unit>)", - "query" - ); - } -} - /** * Validate that all aggregate function names in a list are known. - * Also rejects MRI (Metric Resource Identifier) syntax with guidance - * on the correct tracemetrics query format. * Throws a ValidationError listing valid functions if any are invalid. * * @param aggregates - Parsed aggregate strings (e.g. ["count()", "p95(span.duration)"]) @@ -632,10 +489,6 @@ export function validateAggregateNames( aggregates: string[], dataset?: string ): void { - // Detect MRI syntax early — it passes function-name validation (e.g. "avg" is valid) - // but produces widgets that render as "Internal Error" in the dashboard UI. - rejectMriQueries(aggregates, dataset); - const validFunctions: readonly string[] = dataset === "discover" || dataset === "error-events" ? DISCOVER_AGGREGATE_FUNCTIONS diff --git a/test/commands/dashboard/widget/add.test.ts b/test/commands/dashboard/widget/add.test.ts index 0c77a0c744..d4f3027ce1 100644 --- a/test/commands/dashboard/widget/add.test.ts +++ b/test/commands/dashboard/widget/add.test.ts @@ -212,29 +212,6 @@ describe("dashboard widget add", () => { expect(err.message).toContain("Unknown aggregate function"); }); - test("throws ValidationError for MRI query syntax", async () => { - const { context } = createMockContext(); - const func = await addCommand.loader(); - - const err = await func - .call( - context, - { - json: false, - display: "line", - dataset: "metrics", - query: ["avg(g:custom/node.runtime.mem.rss@byte)"], - }, - "123", - "Memory Usage" - ) - .catch((e: Error) => e); - expect(err).toBeInstanceOf(ValidationError); - expect(err.message).toContain("MRI query syntax is not supported"); - expect(err.message).toContain("tracemetrics"); - expect(err.message).toContain("avg(value,node.runtime.mem.rss,gauge,byte)"); - }); - test("throws ValidationError for big_number with issue dataset", async () => { const { context } = createMockContext(); const func = await addCommand.loader(); diff --git a/test/commands/dashboard/widget/edit.test.ts b/test/commands/dashboard/widget/edit.test.ts index e79ad88501..871dedcc25 100644 --- a/test/commands/dashboard/widget/edit.test.ts +++ b/test/commands/dashboard/widget/edit.test.ts @@ -265,7 +265,7 @@ describe("dashboard widget edit", () => { // Should not throw — "text" is untracked, no dataset constraint applies await func.call( context, - { json: false, index: 0, dataset: "error-events" }, + { json: false, index: 0, dataset: "discover" }, "123" ); expect(updateDashboardSpy).toHaveBeenCalled(); @@ -391,22 +391,22 @@ describe("dashboard widget edit", () => { const { context } = createMockContext(); const func = await editCommand.loader(); - // "failure_rate" is valid for error-events but not spans. - // Here we change the existing spans widget to error-events dataset while - // also setting an error-events-only aggregate. This should succeed. + // "failure_rate" is valid for discover but not spans. + // Here we change the existing spans widget to discover dataset while + // also setting a discover-only aggregate. This should succeed. await func.call( context, { json: false, index: 0, - dataset: "error-events", + dataset: "discover", query: ["failure_rate"], }, "123" ); const body = updateDashboardSpy.mock.calls[0]?.[2]; - expect(body.widgets[0].widgetType).toBe("error-events"); + expect(body.widgets[0].widgetType).toBe("discover"); expect(body.widgets[0].queries[0].aggregates).toEqual(["failure_rate()"]); }); }); diff --git a/test/lib/formatters/human.test.ts b/test/lib/formatters/human.test.ts index 4e826fd070..21c645a0e8 100644 --- a/test/lib/formatters/human.test.ts +++ b/test/lib/formatters/human.test.ts @@ -817,210 +817,4 @@ describe("formatUpgradeResult", () => { expect(result).not.toContain("(from"); }); }); - - test("renders changelog with What's new heading and indented entries", () => { - withPlain(() => { - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.18.0", - targetVersion: "0.21.0", - channel: "stable", - method: "curl", - forced: false, - changelog: { - fromVersion: "0.18.0", - toVersion: "0.21.0", - sections: [ - { - category: "features", - markdown: "- Add dashboard charts\n- Add pagination\n", - }, - { category: "fixes", markdown: "- Fix table rendering\n" }, - ], - totalItems: 3, - truncated: false, - originalCount: 3, - }, - }); - // "What's new" heading with emoji - expect(result).toContain("What's new"); - // Category headings - expect(result).toContain("New Features"); - expect(result).toContain("Bug Fixes"); - // Entries present - expect(result).toContain("Add dashboard charts"); - expect(result).toContain("Fix table rendering"); - // Indented (lines start with spaces) - const changelogStart = result.indexOf("What's new"); - const afterChangelog = result.slice(changelogStart); - const entryLine = afterChangelog - .split("\n") - .find((l: string) => l.includes("Add dashboard charts")); - expect(entryLine).toBeDefined(); - expect(entryLine!.startsWith(" ")).toBe(true); - // Ends with newline - expect(result.endsWith("\n")).toBe(true); - }); - }); - - test("does not render changelog when undefined", () => { - withPlain(() => { - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.5.0", - targetVersion: "0.6.0", - channel: "stable", - method: "curl", - forced: false, - }); - expect(result).not.toContain("What's new"); - }); - }); - - test("does not render changelog when sections are empty", () => { - withPlain(() => { - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.5.0", - targetVersion: "0.6.0", - channel: "stable", - method: "curl", - forced: false, - changelog: { - fromVersion: "0.5.0", - toVersion: "0.6.0", - sections: [], - totalItems: 0, - truncated: false, - originalCount: 0, - }, - }); - expect(result).not.toContain("What's new"); - }); - }); - - test("hides sections with whitespace-only markdown", () => { - withPlain(() => { - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.5.0", - targetVersion: "0.6.0", - channel: "stable", - method: "curl", - forced: false, - changelog: { - fromVersion: "0.5.0", - toVersion: "0.6.0", - sections: [ - { category: "features", markdown: " \n \n" }, - { category: "fixes", markdown: "- Real fix\n" }, - ], - totalItems: 1, - truncated: false, - originalCount: 1, - }, - }); - // Should show fixes but not features - expect(result).toContain("Bug Fixes"); - expect(result).toContain("Real fix"); - expect(result).not.toContain("New Features"); - }); - }); - - test("shows truncation message when changelog is truncated", () => { - withPlain(() => { - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.5.0", - targetVersion: "0.6.0", - channel: "stable", - method: "curl", - forced: false, - changelog: { - fromVersion: "0.5.0", - toVersion: "0.6.0", - sections: [{ category: "features", markdown: "- Feature one\n" }], - totalItems: 1, - truncated: true, - originalCount: 10, - }, - }); - expect(result).toContain("...and 9 more changes"); - expect(result).toContain("github.com/getsentry/cli/releases"); - }); - }); - - test("returns empty changelog when all sections are whitespace", () => { - withPlain(() => { - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.5.0", - targetVersion: "0.6.0", - channel: "stable", - method: "curl", - forced: false, - changelog: { - fromVersion: "0.5.0", - toVersion: "0.6.0", - sections: [ - { category: "features", markdown: " \n" }, - { category: "fixes", markdown: "\n" }, - ], - totalItems: 0, - truncated: false, - originalCount: 0, - }, - }); - expect(result).not.toContain("What's new"); - }); - }); - - test("clamps changelog to terminal height when output would overflow", () => { - // Simulate a tiny terminal to trigger the line-clamping branch. - const origRows = process.stdout.rows; - try { - // Set terminal to 10 rows → maxLines = max(5, floor(10*1.3) - 6) = 7 - Object.defineProperty(process.stdout, "rows", { - value: 10, - writable: true, - configurable: true, - }); - - withPlain(() => { - // Generate many list items to exceed 7 rendered lines - const manyItems = Array.from( - { length: 20 }, - (_, i) => `- Feature number ${i + 1}` - ).join("\n"); - const result = formatUpgradeResult({ - action: "upgraded", - currentVersion: "0.5.0", - targetVersion: "0.6.0", - channel: "stable", - method: "curl", - forced: false, - changelog: { - fromVersion: "0.5.0", - toVersion: "0.6.0", - sections: [{ category: "features", markdown: `${manyItems}\n` }], - totalItems: 20, - truncated: false, - originalCount: 20, - }, - }); - // Should be clamped — shows truncation indicator - expect(result).toContain("...and more"); - expect(result).toContain("github.com/getsentry/cli/releases"); - // Should NOT contain all 20 items - expect(result).not.toContain("Feature number 20"); - }); - } finally { - // Restore original value - Object.defineProperty(process.stdout, "rows", { - value: origRows, - writable: true, - configurable: true, - }); - } - }); }); diff --git a/test/lib/seer-telemetry.test.ts b/test/lib/seer-telemetry.test.ts deleted file mode 100644 index 7293188016..0000000000 --- a/test/lib/seer-telemetry.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -/** - * Tests for Seer telemetry helpers. - * - * Verifies that classifySeerError correctly maps error types to SeerOutcome values. - */ - -import { describe, expect, test } from "bun:test"; -import { ApiError, SeerError, TimeoutError } from "../../src/lib/errors.js"; -import { - classifySeerError, - type SeerOutcome, -} from "../../src/lib/telemetry.js"; - -describe("classifySeerError", () => { - test("maps SeerError no_budget", () => { - expect(classifySeerError(new SeerError("no_budget", "my-org"))).toBe( - "no_budget" satisfies SeerOutcome - ); - }); - - test("maps SeerError not_enabled", () => { - expect(classifySeerError(new SeerError("not_enabled"))).toBe( - "not_enabled" satisfies SeerOutcome - ); - }); - - test("maps SeerError ai_disabled", () => { - expect(classifySeerError(new SeerError("ai_disabled", "org"))).toBe( - "ai_disabled" satisfies SeerOutcome - ); - }); - - test("maps TimeoutError", () => { - expect(classifySeerError(new TimeoutError("timed out"))).toBe( - "timeout" satisfies SeerOutcome - ); - }); - - test("maps ApiError", () => { - expect(classifySeerError(new ApiError("not found", 404))).toBe( - "api_error" satisfies SeerOutcome - ); - }); - - test("maps generic Error", () => { - expect(classifySeerError(new Error("broke"))).toBe( - "error" satisfies SeerOutcome - ); - }); - - test("maps non-Error values", () => { - expect(classifySeerError("string")).toBe("error" satisfies SeerOutcome); - expect(classifySeerError(null)).toBe("error" satisfies SeerOutcome); - expect(classifySeerError(undefined)).toBe("error" satisfies SeerOutcome); - }); -}); diff --git a/test/skill-eval/cases.json b/test/skill-eval/cases.json new file mode 100644 index 0000000000..e80c67b5fc --- /dev/null +++ b/test/skill-eval/cases.json @@ -0,0 +1,179 @@ +[ + { + "id": "list-issues-basic", + "prompt": "show me the sentry issues", + "description": "Basic issue listing — should auto-detect org/project", + "criteria": { + "no-pre-auth": { + "brief": "Does not run auth commands before the actual command", + "anti-patterns": [ + "sentry auth login", + "sentry auth status", + "sentry auth whoami" + ] + }, + "no-pre-lookup": { + "brief": "Does not run org/project listing before the actual command", + "anti-patterns": [ + "sentry org list", + "sentry project list", + "sentry org view", + "sentry project view" + ] + }, + "correct-command": { + "brief": "Uses sentry issue list as the primary command", + "expected-patterns": ["sentry issue list"] + }, + "minimal-calls": { + "brief": "Plans at most 2 CLI commands total", + "max-commands": 2 + }, + "trusts-auto-detect": { + "brief": "Does not hardcode org/project names", + "anti-patterns": ["my-org/", "my-project"] + } + } + }, + { + "id": "recent-errors", + "prompt": "what errors happened recently in my project?", + "description": "Recent error lookup — should use issue list, auto-detect context", + "criteria": { + "no-pre-auth": { + "brief": "Does not run auth commands before the actual command", + "anti-patterns": ["sentry auth login", "sentry auth status"] + }, + "no-pre-lookup": { + "brief": "Does not run org/project listing before the actual command", + "anti-patterns": ["sentry org list", "sentry project list"] + }, + "correct-command": { + "brief": "Uses sentry issue list as the primary command", + "expected-patterns": ["sentry issue list"] + }, + "minimal-calls": { + "brief": "Plans at most 2 CLI commands total", + "max-commands": 2 + } + } + }, + { + "id": "view-specific-issue", + "prompt": "tell me about issue PROJ-42", + "description": "View a specific issue by short ID — single command", + "criteria": { + "no-pre-auth": { + "brief": "Does not pre-authenticate", + "anti-patterns": ["sentry auth login", "sentry auth status"] + }, + "correct-command": { + "brief": "Uses sentry issue view with the correct short ID", + "expected-patterns": ["sentry issue view", "PROJ-42"] + }, + "minimal-calls": { + "brief": "Plans exactly 1 CLI command", + "max-commands": 1 + } + } + }, + { + "id": "explain-issue", + "prompt": "why is CLI-G5 happening? can you analyze the root cause?", + "description": "Root cause analysis — view + explain is acceptable (2 commands max)", + "criteria": { + "no-pre-auth": { + "brief": "Does not pre-authenticate", + "anti-patterns": ["sentry auth login", "sentry auth status"] + }, + "correct-command": { + "brief": "Uses sentry issue explain with the correct ID", + "expected-patterns": ["sentry issue explain", "CLI-G5"] + }, + "minimal-calls": { + "brief": "Plans at most 2 CLI commands", + "max-commands": 2 + } + } + }, + { + "id": "list-traces", + "prompt": "show me recent traces for performance analysis", + "description": "Trace listing — should use trace list with auto-detection. Second command (if any) could be a follow-up like trace view.", + "criteria": { + "no-pre-auth": { + "brief": "Does not pre-authenticate", + "anti-patterns": ["sentry auth login", "sentry auth status"] + }, + "no-pre-lookup": { + "brief": "Does not look up org/project first", + "anti-patterns": ["sentry org list", "sentry project list"] + }, + "correct-command": { + "brief": "Uses sentry trace list", + "expected-patterns": ["sentry trace list"] + }, + "minimal-calls": { + "brief": "Plans at most 2 CLI commands", + "max-commands": 2 + }, + "trusts-auto-detect": { + "brief": "Does not hardcode org/project names", + "anti-patterns": ["my-org/", "my-project"] + } + } + }, + { + "id": "json-output", + "prompt": "get me all unresolved issues as JSON so I can process them in a script", + "description": "Machine-readable output — should use --json flag", + "criteria": { + "correct-command": { + "brief": "Uses sentry issue list with --json flag", + "expected-patterns": ["sentry issue list", "--json"] + }, + "minimal-calls": { + "brief": "Plans at most 2 CLI commands", + "max-commands": 2 + }, + "trusts-auto-detect": { + "brief": "Does not hardcode org/project names", + "anti-patterns": ["my-org/", "my-project"] + } + } + }, + { + "id": "api-fallback", + "prompt": "I need to get the raw organization settings from the Sentry API", + "description": "API call — should use sentry api, not external curl", + "criteria": { + "correct-command": { + "brief": "Uses sentry api for the request", + "expected-patterns": ["sentry api"] + }, + "no-external-curl": { + "brief": "Does not use raw curl commands", + "anti-patterns": ["curl "] + } + } + }, + { + "id": "explore-schema", + "prompt": "what API endpoints are available for releases?", + "description": "Schema exploration — should use sentry schema", + "criteria": { + "correct-command": { + "brief": "Uses sentry schema to explore the API", + "expected-patterns": ["sentry schema"] + }, + "no-external-fetch": { + "brief": "Does not fetch external API docs", + "anti-patterns": ["docs.sentry.io/api", "openapi", "swagger"] + }, + "minimal-calls": { + "brief": "Plans at most 2 CLI commands", + "max-commands": 2 + } + } + } +] diff --git a/test/skill-eval/helpers/judge.ts b/test/skill-eval/helpers/judge.ts new file mode 100644 index 0000000000..3afe999129 --- /dev/null +++ b/test/skill-eval/helpers/judge.ts @@ -0,0 +1,199 @@ +/** + * Phase 2: Grade the agent's plan against test case criteria. + * + * Two passes: + * 1. Deterministic — string matching for anti-patterns, expected-patterns, max-commands + * 2. LLM judge — coherence/quality check using a cheap model (Haiku 4.6) + */ + +import type { LLMClient } from "./llm-client.js"; +import { chatCompletion } from "./llm-client.js"; +import type { + AgentPlan, + CaseResult, + CriterionDef, + CriterionResult, + TestCase, +} from "./types.js"; + +/** + * Evaluate a single deterministic criterion against the plan's commands. + * Returns a pass/fail result with a human-readable reason. + */ +function evaluateDeterministic( + name: string, + def: CriterionDef, + plan: AgentPlan +): CriterionResult { + const allCommands = plan.commands.map((c) => c.command.toLowerCase()); + + // Check anti-patterns: none of these strings should appear in any command + if (def["anti-patterns"]) { + for (const pattern of def["anti-patterns"]) { + const found = allCommands.find((cmd) => + cmd.includes(pattern.toLowerCase()) + ); + if (found) { + return { + name, + pass: false, + reason: `Found anti-pattern '${pattern}' in: ${found}`, + }; + } + } + } + + // Check expected patterns: at least one command must contain each pattern + if (def["expected-patterns"]) { + for (const pattern of def["expected-patterns"]) { + const lowerPattern = pattern.toLowerCase(); + if (!allCommands.some((cmd) => cmd.includes(lowerPattern))) { + return { + name, + pass: false, + reason: `Expected pattern '${pattern}' not found in any command`, + }; + } + } + } + + // Check max commands + if ( + def["max-commands"] !== undefined && + plan.commands.length > def["max-commands"] + ) { + return { + name, + pass: false, + reason: `Too many commands: ${plan.commands.length} (max: ${def["max-commands"]})`, + }; + } + + return { name, pass: true, reason: def.brief }; +} + +/** + * Use the LLM judge to evaluate overall plan quality. + * Returns null if the judge call fails. + */ +async function evaluateWithLLMJudge( + client: LLMClient, + prompt: string, + plan: AgentPlan +): Promise<CriterionResult> { + const commandList = plan.commands + .map((c, i) => `${i + 1}. \`${c.command}\` — ${c.purpose}`) + .join("\n"); + + const judgePrompt = `You are evaluating whether an AI agent's CLI command plan is good. + +The user asked: "${prompt}" + +The agent's plan: +Thinking: ${plan.thinking} +Commands: +${commandList} +Notes: ${plan.notes} + +Evaluate the plan on overall quality. A good plan: +- Uses the right Sentry CLI commands for the task +- Would actually work if executed +- Is efficient (no unnecessary commands) +- Directly addresses what the user asked for + +Return ONLY valid JSON: +{"pass": true, "reason": "Brief explanation"} + +or + +{"pass": false, "reason": "Brief explanation of what's wrong"}`; + + try { + const text = await chatCompletion( + client, + client.judgeModel, + [{ role: "user", content: judgePrompt }], + 512 + ); + + const jsonMatch = text.match(/\{[\s\S]*\}/); + if (!jsonMatch) { + return { + name: "overall-quality", + pass: false, + reason: "Judge failed to return valid JSON", + }; + } + + const parsed = JSON.parse(jsonMatch[0]) as { + pass: boolean; + reason: string; + }; + return { + name: "overall-quality", + pass: parsed.pass === true, + reason: parsed.reason, + }; + } catch (err) { + return { + name: "overall-quality", + pass: false, + reason: `Judge error: ${err instanceof Error ? err.message : String(err)}`, + }; + } +} + +/** + * Evaluate a test case's plan against all its criteria. + * Runs deterministic checks first, then the LLM judge for overall quality. + */ +export async function judgePlan( + client: LLMClient, + testCase: TestCase, + plan: AgentPlan | null +): Promise<CaseResult> { + // If the planner failed to produce a plan, fail all criteria + if (!plan) { + const criteria = Object.keys(testCase.criteria).map((name) => ({ + name, + pass: false, + reason: "No plan generated — planner failed", + })); + criteria.push({ + name: "overall-quality", + pass: false, + reason: "No plan generated — planner failed", + }); + return { + caseId: testCase.id, + prompt: testCase.prompt, + plan: null, + criteria, + score: 0, + passed: false, + }; + } + + // Run deterministic checks + const criteria: CriterionResult[] = []; + for (const [name, def] of Object.entries(testCase.criteria)) { + criteria.push(evaluateDeterministic(name, def, plan)); + } + + // Run LLM judge for overall quality + const llmVerdict = await evaluateWithLLMJudge(client, testCase.prompt, plan); + criteria.push(llmVerdict); + + // Compute score: fraction of criteria that passed + const passing = criteria.filter((c) => c.pass).length; + const score = criteria.length > 0 ? passing / criteria.length : 0; + + return { + caseId: testCase.id, + prompt: testCase.prompt, + plan, + criteria, + score, + passed: criteria.every((c) => c.pass), + }; +} diff --git a/test/skill-eval/helpers/llm-client.ts b/test/skill-eval/helpers/llm-client.ts new file mode 100644 index 0000000000..4bdeb7adc2 --- /dev/null +++ b/test/skill-eval/helpers/llm-client.ts @@ -0,0 +1,64 @@ +/** + * LLM client for the skill eval framework. + * + * Uses the Anthropic API via @anthropic-ai/sdk (already in devDependencies). + * Requires ANTHROPIC_API_KEY env var. + */ + +/** Default agent models — the target models for the skill */ +export const DEFAULT_AGENT_MODELS = ["claude-sonnet-4-6", "claude-opus-4-6"]; + +/** Default judge model — cheap and fast, just needs to grade command plans */ +export const DEFAULT_JUDGE_MODEL = "claude-haiku-4-5-20251001"; + +export type LLMClient = { + client: InstanceType<typeof import("@anthropic-ai/sdk").default>; + agentModels: string[]; + judgeModel: string; +}; + +/** + * Create an LLM client for the eval framework. + * + * Agent models and judge model can be overridden via env vars: + * - EVAL_AGENT_MODELS: comma-separated list of model IDs + * - EVAL_JUDGE_MODEL: single model ID + */ +export async function createClient(apiKey: string): Promise<LLMClient> { + const { default: Anthropic } = await import("@anthropic-ai/sdk"); + + const client = new Anthropic({ apiKey }); + + const agentModels = process.env.EVAL_AGENT_MODELS + ? process.env.EVAL_AGENT_MODELS.split(",").map((m) => m.trim()) + : DEFAULT_AGENT_MODELS; + + const judgeModel = process.env.EVAL_JUDGE_MODEL ?? DEFAULT_JUDGE_MODEL; + + return { client, agentModels, judgeModel }; +} + +/** Send a message and return the text response */ +export async function chatCompletion( + llm: LLMClient, + model: string, + messages: { role: "system" | "user"; content: string }[], + maxTokens = 2048 +): Promise<string> { + // Separate system prompt from user messages (Anthropic API style) + const systemMsg = messages.find((m) => m.role === "system"); + const userMsgs = messages + .filter((m) => m.role === "user") + .map((m) => ({ role: "user" as const, content: m.content })); + + const response = await llm.client.messages.create({ + model, + max_tokens: maxTokens, + system: systemMsg?.content, + messages: userMsgs, + }); + + // Extract text from content blocks + const textBlock = response.content.find((b) => b.type === "text"); + return textBlock?.text ?? ""; +} diff --git a/test/skill-eval/helpers/planner.ts b/test/skill-eval/helpers/planner.ts new file mode 100644 index 0000000000..805e358b39 --- /dev/null +++ b/test/skill-eval/helpers/planner.ts @@ -0,0 +1,90 @@ +/** + * Phase 1: Send SKILL.md + user prompt to the agent model. + * + * The agent is framed as an AI coding assistant with terminal access. + * It must plan which CLI commands to run, outputting structured JSON. + */ + +import type { LLMClient } from "./llm-client.js"; +import { chatCompletion } from "./llm-client.js"; +import type { AgentPlan } from "./types.js"; + +/** + * Build the system prompt that frames the LLM as an agent with the skill loaded. + * The SKILL.md content is injected directly so the model plans based on it. + */ +function buildSystemPrompt(skillContent: string): string { + return `You are an AI coding agent helping a developer. You have access to a terminal where you can run CLI commands. + +The developer is working in a project that uses Sentry for error tracking and monitoring. The Sentry CLI is installed. Here is your guide for using it: + +<skill> +${skillContent} +</skill> + +When the developer asks you to do something, plan which CLI commands you would run. +Output your plan as JSON with this exact structure: +{ + "thinking": "Brief reasoning about what the user wants and how to accomplish it", + "commands": [ + { + "command": "the exact CLI command you would run", + "purpose": "why you are running this command" + } + ], + "notes": "Any caveats or follow-up suggestions for the user" +} + +Rules: +- Output ONLY the JSON object, no markdown fencing, no extra text +- List commands in the order you would execute them +- Be specific with flag values — use actual values, not placeholders +- If the skill guide says something works automatically (auth, org/project detection), trust it +- Do NOT run commands just to gather information you don't need for the task`; +} + +/** + * Generate a command plan from an agent model given a user prompt. + * Returns the parsed plan, or null if parsing fails. + */ +export async function generatePlan( + client: LLMClient, + model: string, + skillContent: string, + userPrompt: string +): Promise<AgentPlan | null> { + const systemPrompt = buildSystemPrompt(skillContent); + + let text: string; + try { + text = await chatCompletion(client, model, [ + { role: "system", content: systemPrompt }, + { role: "user", content: userPrompt }, + ]); + } catch (err) { + console.error( + ` [planner] API error: ${err instanceof Error ? err.message : String(err)}` + ); + return null; + } + + // Extract JSON from response (handle potential markdown fencing) + const jsonMatch = text.match(/\{[\s\S]*\}/); + if (!jsonMatch) { + console.error(" [planner] Failed to extract JSON from response"); + console.error(` Response: ${text.slice(0, 300)}`); + return null; + } + + try { + const parsed = JSON.parse(jsonMatch[0]) as AgentPlan; + // Normalize: ensure commands is an array + if (!Array.isArray(parsed.commands)) { + parsed.commands = []; + } + return parsed; + } catch { + console.error(` [planner] Invalid JSON: ${jsonMatch[0].slice(0, 300)}`); + return null; + } +} diff --git a/test/skill-eval/helpers/report.ts b/test/skill-eval/helpers/report.ts new file mode 100644 index 0000000000..c5221188ac --- /dev/null +++ b/test/skill-eval/helpers/report.ts @@ -0,0 +1,73 @@ +/** + * Format and output eval results to console and JSON file. + */ + +import type { CaseResult, EvalReport, ModelResult } from "./types.js"; + +/** Format a single case result as console lines */ +function formatCaseResult(c: CaseResult): string { + const lines: string[] = []; + const icon = c.passed ? " ✓" : " ✗"; + const scorePct = (c.score * 100).toFixed(0); + lines.push(`${icon} ${c.caseId} (${scorePct}%)`); + + for (const cr of c.criteria) { + const crIcon = cr.pass ? " PASS" : " FAIL"; + lines.push(`${crIcon} ${cr.name}: ${cr.reason}`); + } + return lines.join("\n"); +} + +/** Format a single model result as console lines */ +function formatModelResult(model: ModelResult, threshold: number): string { + const lines: string[] = []; + const pct = (model.score * 100).toFixed(1); + const status = model.score >= threshold ? "PASS" : "FAIL"; + lines.push( + `${status} ${model.model}: ${model.totalPassed}/${model.totalCases} cases (${pct}%)` + ); + lines.push(""); + + for (const c of model.cases) { + lines.push(formatCaseResult(c)); + lines.push(""); + } + return lines.join("\n"); +} + +/** Print a console report of all model results */ +export function printReport(report: EvalReport): void { + console.log(""); + console.log("Skill Eval Results"); + console.log("══════════════════"); + console.log(`Threshold: ${(report.threshold * 100).toFixed(0)}%`); + console.log(""); + + for (const model of report.models) { + console.log(formatModelResult(model, report.threshold)); + } + + // Summary + console.log("══════════════════"); + for (const model of report.models) { + const pct = (model.score * 100).toFixed(1); + const status = model.score >= report.threshold ? "PASS" : "FAIL"; + console.log(`${status} ${model.model}: ${pct}%`); + } + console.log(""); +} + +/** Write the full eval report to a JSON file */ +export async function writeJsonReport( + report: EvalReport, + path: string +): Promise<void> { + await Bun.write(path, JSON.stringify(report, null, 2)); + console.log(`Results written to ${path}`); +} + +/** Check if all models meet the threshold, return appropriate exit code */ +export function getExitCode(report: EvalReport): number { + const allPass = report.models.every((m) => m.score >= report.threshold); + return allPass ? 0 : 1; +} diff --git a/test/skill-eval/helpers/types.ts b/test/skill-eval/helpers/types.ts new file mode 100644 index 0000000000..3caa5b2743 --- /dev/null +++ b/test/skill-eval/helpers/types.ts @@ -0,0 +1,73 @@ +/** + * Shared types for the skill evaluation framework. + * + * The eval tests whether SKILL.md effectively guides an LLM to use the + * Sentry CLI efficiently — no pre-auth, no org lookup, correct fields, + * minimal tool calls. + */ + +/** A single planned CLI command from the agent under test */ +export type PlannedCommand = { + command: string; + purpose: string; +}; + +/** Structured plan output from the agent under test */ +export type AgentPlan = { + thinking: string; + commands: PlannedCommand[]; + notes: string; +}; + +/** Criterion definition from cases.json */ +export type CriterionDef = { + brief: string; + /** Strings that must NOT appear in any command */ + "anti-patterns"?: string[]; + /** Strings that MUST appear in at least one command */ + "expected-patterns"?: string[]; + /** Maximum number of commands allowed */ + "max-commands"?: number; +}; + +/** Test case definition from cases.json */ +export type TestCase = { + id: string; + prompt: string; + description: string; + criteria: Record<string, CriterionDef>; +}; + +/** Result of evaluating a single criterion */ +export type CriterionResult = { + name: string; + pass: boolean; + reason: string; +}; + +/** Result of evaluating one test case against one model */ +export type CaseResult = { + caseId: string; + prompt: string; + plan: AgentPlan | null; + criteria: CriterionResult[]; + score: number; + passed: boolean; +}; + +/** Results for all cases run against one model */ +export type ModelResult = { + model: string; + cases: CaseResult[]; + totalPassed: number; + totalCases: number; + score: number; +}; + +/** Full eval report written to results.json */ +export type EvalReport = { + timestamp: string; + skillVersion: string; + threshold: number; + models: ModelResult[]; +}; diff --git a/test/types/dashboard.test.ts b/test/types/dashboard.test.ts index 5d3af2597f..63e4b2818a 100644 --- a/test/types/dashboard.test.ts +++ b/test/types/dashboard.test.ts @@ -8,12 +8,10 @@ import { describe, expect, test } from "bun:test"; import { ValidationError } from "../../src/lib/errors.js"; import { - ALL_WIDGET_TYPES, assignDefaultLayout, type DashboardWidget, DashboardWidgetInputSchema, DEFAULT_WIDGET_TYPE, - DEPRECATED_WIDGET_TYPES, DISCOVER_AGGREGATE_FUNCTIONS, DISPLAY_TYPES, DiscoverAggregateFunctionSchema, @@ -26,7 +24,6 @@ import { IsFilterValueSchema, mapWidgetTypeToDataset, parseAggregate, - parseMri, parseSortExpression, parseWidgetInput, prepareWidgetQueries, @@ -35,7 +32,6 @@ import { stripWidgetServerFields, TABLE_DISPLAY_TYPES, TIMESERIES_DISPLAY_TYPES, - validateAggregateNames, validateWidgetLayout, WIDGET_TYPES, type WidgetType, @@ -51,13 +47,7 @@ describe("WIDGET_TYPES", () => { expect(DEFAULT_WIDGET_TYPE).toBe("spans"); }); - test("excludes deprecated types", () => { - for (const t of DEPRECATED_WIDGET_TYPES) { - expect(WIDGET_TYPES).not.toContain(t); - } - }); - - test("ALL_WIDGET_TYPES includes both active and deprecated", () => { + test("contains all expected dataset types", () => { const expected: WidgetType[] = [ "discover", "issue", @@ -70,7 +60,7 @@ describe("WIDGET_TYPES", () => { "preprod-app-size", ]; for (const t of expected) { - expect(ALL_WIDGET_TYPES).toContain(t); + expect(WIDGET_TYPES).toContain(t); } }); }); @@ -889,174 +879,3 @@ describe("display type sets", () => { expect(TABLE_DISPLAY_TYPES.has("line")).toBe(false); }); }); - -// --------------------------------------------------------------------------- -// parseMri -// --------------------------------------------------------------------------- - -describe("parseMri", () => { - test("parses standard gauge MRI", () => { - expect(parseMri("g:custom/node.runtime.mem.rss@byte")).toEqual({ - entity: "g", - namespace: "custom", - name: "node.runtime.mem.rss", - unit: "byte", - }); - }); - - test("parses counter MRI", () => { - expect(parseMri("c:transactions/measurements.db_calls@none")).toEqual({ - entity: "c", - namespace: "transactions", - name: "measurements.db_calls", - unit: "none", - }); - }); - - test("parses distribution MRI", () => { - expect( - parseMri("d:transactions/measurements.stall_longest_time@millisecond") - ).toEqual({ - entity: "d", - namespace: "transactions", - name: "measurements.stall_longest_time", - unit: "millisecond", - }); - }); - - test("parses set MRI", () => { - expect(parseMri("s:sessions/error@none")).toEqual({ - entity: "s", - namespace: "sessions", - name: "error", - unit: "none", - }); - }); - - test("parses extracted MRI", () => { - expect(parseMri("e:spans/duration@millisecond")).toEqual({ - entity: "e", - namespace: "spans", - name: "duration", - unit: "millisecond", - }); - }); - - test("parses multi-char entity (matches canonical Python behavior)", () => { - expect(parseMri("dist:my_namespace/foo@none")).toEqual({ - entity: "dist", - namespace: "my_namespace", - name: "foo", - unit: "none", - }); - }); - - test("handles units with slashes", () => { - expect(parseMri("d:transactions/measurements.disk_io@byte/second")).toEqual( - { - entity: "d", - namespace: "transactions", - name: "measurements.disk_io", - unit: "byte/second", - } - ); - }); - - test("returns null for non-MRI strings", () => { - expect(parseMri("span.duration")).toBeNull(); - expect(parseMri("count()")).toBeNull(); - expect(parseMri("value,name,gauge,byte")).toBeNull(); - }); - - test("returns null for partial MRI (missing unit)", () => { - expect(parseMri("g:custom/foo")).toBeNull(); - }); - - test("returns null for empty string", () => { - expect(parseMri("")).toBeNull(); - }); - - test("returns null for malformed separators", () => { - expect(parseMri("d@transactions/foo")).toBeNull(); - expect(parseMri(":transactions/foo@none")).toBeNull(); - expect(parseMri("d/transactions@foo:none")).toBeNull(); - expect(parseMri(":/@")).toBeNull(); - }); -}); - -// --------------------------------------------------------------------------- -// validateAggregateNames — MRI rejection -// --------------------------------------------------------------------------- - -describe("validateAggregateNames MRI rejection", () => { - test("throws ValidationError for MRI query with metrics dataset", () => { - expect(() => - validateAggregateNames( - ["avg(g:custom/node.runtime.mem.rss@byte)"], - "metrics" - ) - ).toThrow(ValidationError); - }); - - test("throws ValidationError for MRI query with spans dataset", () => { - expect(() => - validateAggregateNames(["avg(g:custom/node.runtime.mem.rss@byte)"]) - ).toThrow(ValidationError); - }); - - test("error message contains tracemetrics format suggestion", () => { - try { - validateAggregateNames( - ["avg(g:custom/node.runtime.mem.rss@byte)"], - "metrics" - ); - expect.unreachable("Should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - const msg = (error as ValidationError).message; - expect(msg).toContain("avg(value,node.runtime.mem.rss,gauge,byte)"); - expect(msg).toContain("tracemetrics"); - } - }); - - test("error message suggests --dataset tracemetrics when dataset is metrics", () => { - try { - validateAggregateNames( - ["avg(g:custom/node.runtime.mem.rss@byte)"], - "metrics" - ); - expect.unreachable("Should have thrown"); - } catch (error) { - const msg = (error as ValidationError).message; - expect(msg).toContain( - "Use --dataset tracemetrics instead of --dataset metrics" - ); - } - }); - - test("error message omits dataset switch when already tracemetrics", () => { - try { - validateAggregateNames( - ["avg(g:custom/node.runtime.mem.rss@byte)"], - "tracemetrics" - ); - expect.unreachable("Should have thrown"); - } catch (error) { - const msg = (error as ValidationError).message; - expect(msg).not.toContain("Use --dataset tracemetrics instead"); - } - }); - - test("does not throw for valid non-MRI queries", () => { - expect(() => validateAggregateNames(["avg(span.duration)"])).not.toThrow(); - expect(() => validateAggregateNames(["count()"])).not.toThrow(); - expect(() => validateAggregateNames(["p95(span.self_time)"])).not.toThrow(); - }); - - test("does not throw for tracemetrics format query", () => { - // tracemetrics format: fn(value,name,type,unit) — not MRI - expect(() => - validateAggregateNames(["avg(value,node.runtime.mem.rss,gauge,byte)"]) - ).not.toThrow(); - }); -});