Skip to content

feat: per-stage telemetry in JSON output and full stage visibility in --debug - #207

Merged
Tgenz1213 merged 8 commits into
mainfrom
feat/201-stage-telemetry
Sep 20, 2026
Merged

Tgenz1213 merged 8 commits into
mainfrom
feat/201-stage-telemetry

Conversation

@Tgenz1213

Copy link
Copy Markdown
Owner

Summary

Each ranking stage now reports what it received, kept and how long it took: as a new stages array in check --format json, and per stage and file in --debug. This lets users measure what each stage saves and costs so they can tune the pipeline.

Related issue

Closes #201

In scope

  • stage.Telemetry (new, internal/analysis/stage/telemetry.go) runs the stages and totals received, kept and time per stage, safe for concurrent files.
  • check --format json gains a top-level stages array of {name, received, kept, duration_ms}, one entry per stage in pipeline order, including the default rank stage. Existing fields (violations, count, failures) are unchanged.
  • --debug prints, per stage and file, Stage <name>: N candidate(s) received, M kept, a Kept: line with its score for each survivor, and a reason line (Below threshold or Cut by top-K limit) for every dropped candidate.
  • README documents the JSON fields; CLAUDE.md and ADRs 0014 and 0022 describe the design.

Out of scope

  • Token-usage reporting per stage (out of scope in the epic).
  • Telemetry in text output; default text output is unchanged.

Architectural notes

Accepted deviations from "identical behavior":

  • --debug no longer caps the Below threshold list at MaxKeep. The issue asks for every dropped candidate with its reason, so all of them are listed. Debug output grows on large corpora.
  • A stage handed no candidates is not scored (Stage.Apply returns before calling its scorer). A file with no scope-matched ADRs therefore makes no embedding call, and its stages report zeros including duration_ms, as the issue requires. Consequence: with on_error: fail, an embedding outage is no longer detected for a file that has no candidates (main exited 6 for it; this branch exits 0 with a clean run).
  • duration_ms is summed per file, so with concurrent files it can exceed wall-clock time. Stated in the README.

Adapted to #206 (on_error) during rebase: stage naming uses #206's Stage.Name, and the report carries both stages and failures. A stage that fails (either on_error mode) still appears in stages with its received count and time and kept: 0.

No new ADR: the design amends ADR 0022 (pipeline) and ADR 0014 (JSON document).

Follow-ups

  • The rank and rerank stage names are string literals in internal/analysis/stages.go and internal/analysis/stage/cosine.go; constants would be tidier.

QA / test plan

Verified locally:

  • go test ./... passes, including Docker-backed Postgres tests; go test -race on internal/analysis/... and internal/cli/... in a Linux container; containerised golangci-lint reports 0 issues; gofmt is clean; each commit builds and vets on its own.
  • Differential run of main (38b879a) against this branch using the mock-provider binary: 96 runs per binary across 4 pipelines (none, rank+rerank, rerank-only, on_error: fail), violating/clean/embed-failure files, scope-matching and non-matching ADRs, and text, --format json, --debug and --format json --debug modes. Text output without --debug is byte-identical for files with candidates; JSON differs only by the new stages field; exit codes and the LLM cache contents are identical. The only other differences are the new --debug lines and the skip-empty change above.
  • Mutation check (30 mutations of the new behavior) run before the rebase; the three surviving mutations now have tests. Not re-run after the rebase.

Not verified here, needs CI or follow-up: --suggest-fixes, baseline mode, --staged and --all with telemetry; embedding-call counts under a single-provider config.

Checklist

  • Title follows Conventional Commits (feat:, fix:, docs:, refactor:, build(deps):, etc.)
  • All acceptance criteria from the linked issue are met
  • go test -race -cover ./... passes (race run locally on the changed packages only; the full race run is left to CI)
  • golangci-lint run --timeout=5m is clean
  • CLAUDE.md updated if this changes build/test commands, adds or renames a top-level package, changes a cross-package interface, or adds a footgun
  • A new ADR added under docs/arch/ if this embodies an architecturally-significant decision (existing ADRs 0014 and 0022 amended instead)
  • Comments follow the 2-line-max, WHY-only convention

🤖 Generated with Claude Code

Tgenz1213 and others added 5 commits September 20, 2026 16:19
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ages

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 20, 2026 21:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Empty-input stage timing contradicts the documented contract, and the stale guidance should be corrected.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds per-stage telemetry to JSON output and expanded stage diagnostics in --debug.

Changes:

  • Adds concurrent-safe stage counts and timing.
  • Reports kept and dropped candidates with reasons.
  • Updates CLI, documentation, and test coverage.
File Summary
test/​e2e_test.go Extends end-to-end JSON coverage.
test/​e2e_pipeline_test.go Verifies stage totals end to end.
README.md Documents stage telemetry fields.
internal/​cli/​report_test.go Tests report serialization.
internal/​cli/​cli.go Emits stage statistics in JSON.
internal/​cli/​cli_test.go Tests CLI telemetry output.
internal/​analysis/​stages_test.go Tests stage pipeline behavior.
internal/​analysis/​stage/​telemetry.go Aggregates per-stage statistics; empty-input timing requires correction.
internal/​analysis/​stage/​telemetry_test.go Tests telemetry aggregation.
internal/​analysis/​stage/​stage.go Adds detailed stage debug output.
internal/​analysis/​stage/​stage_test.go Tests stage behavior and diagnostics.
internal/​analysis/​engine.go Integrates telemetry into pipeline execution.
internal/​analysis/​engine_stage_stats_test.go Tests engine stage statistics.
internal/​analysis/​analysis_test.go Covers analysis pipeline behavior.
docs/​arch/​0022-candidate-scoring-pipeline.md Documents telemetry and debug behavior.
docs/​arch/​0014-json-check-output.md Documents JSON stage output.
CLAUDE.md Updates guidance; obsolete debug cap wording remains.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/analysis/stage/telemetry.go
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

JSON reports omit configured stages when file discovery fails, and the documented timing semantics do not match the implementation.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Include zero-valued stage telemetry when file discovery fails

internal/​analysis/​engine.go:146

Telemetry is created only after Content.GetFiles succeeds. runCheck still emits the JSON report when Run returns that error, so this path produces "stages": [] rather than one zero-valued entry per configured stage, violating the requirement that every pipeline stage be listed. Initialize the stage list/telemetry before file discovery and snapshot its zero stats on this early return.

Low severity Document duration_ms as total stage-application time

README.md:332

This describes duration_ms as scorer time, but Telemetry.Apply measures the entire Stage.Apply call, including thresholding, sorting, and generation of all debug lines. Since this metric is intended for cost tuning and can change under --debug, document it as total stage-application time rather than scoring time.

Tgenz1213 and others added 2 commits September 20, 2026 16:52
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Tgenz1213

Copy link
Copy Markdown
Owner Author

Both findings from the follow-up review were valid and are fixed and pushed (f8d4e5d, c773454).

  • Stages on a file-discovery failure: Engine.Run now builds the stage list and telemetry before calling GetFiles. On a discovery error it sets CollectedStages (under JSONOutput) to the zero-valued stats, so the JSON report that runCheck still writes lists every configured stage instead of "stages": []. TestEngine_ListsEveryStageWhenFileDiscoveryFails covers it.
  • duration_ms wording: the README now describes it as the total time spent applying the stage (scoring, thresholding and, under --debug, writing its debug output), since Telemetry.Apply times the whole Stage.Apply call. ADR 0022 already said 'time spent in Apply', so no change was needed there.

Lint, gofmt and go test -race on the changed packages pass locally; CI is re-running.

@Tgenz1213
Tgenz1213 merged commit ddc36d7 into main Sep 20, 2026
7 checks passed
@Tgenz1213
Tgenz1213 deleted the feat/201-stage-telemetry branch September 20, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: per-stage telemetry in JSON output and full stage visibility in --debug

2 participants