Add ambient context metric to audit and logs commands#27137
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/70fc852c-c992-4b32-87f6-ebb89735311d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/70fc852c-c992-4b32-87f6-ebb89735311d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an “ambient context” token metric (capturing the first LLM invocation’s input/cached/effective token footprint) and exposes it in gh aw audit and gh aw logs JSON outputs, with accompanying unit tests and documentation updates.
Changes:
- Parse and compute
ambient_contextfrom the earliest (chronological) token-usage entry and attach it toTokenUsageSummary. - Expose
ambient_contextin audit metrics output and per-run logs output. - Add unit tests for parsing/serialization and update reference docs.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/token_usage.go | Adds AmbientContextMetrics, computes ambient context from first chronological invocation, and includes it in TokenUsageSummary. |
| pkg/cli/token_usage_test.go | Adds tests validating ambient context extraction and default behavior when cached tokens are absent. |
| pkg/cli/logs_report.go | Extends logs JSON schema to include per-run ambient_context sourced from token usage summary. |
| pkg/cli/logs_ambient_context_test.go | Verifies buildLogsData includes ambient context in structured logs output. |
| pkg/cli/audit_report.go | Extends audit metrics JSON schema to include ambient_context and populates it from token usage. |
| pkg/cli/audit_ambient_context_test.go | Verifies buildAuditData includes ambient context in audit metrics. |
| docs/src/content/docs/reference/audit.md | Documents ambient_context in audit metrics and logs JSON output. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/cli/token_usage.go:112
- parseTokenUsageFile now buffers every parsed TokenUsageEntry into an in-memory slice so it can sort to find the first chronological invocation. This makes parsing O(n) memory + O(n log n) time and can get expensive for large token-usage.jsonl files. Consider computing the “first” ambient-context entry in a single pass while scanning (track the earliest valid timestamp / fallback to first-seen order) so totals can still be aggregated streaming without retaining/sorting the full dataset.
entries := make([]TokenUsageEntry, 0)
lineNum := 0
for scanner.Scan() {
lineNum++
line := strings.TrimSpace(scanner.Text())
- Files reviewed: 7/7 changed files
- Comments generated: 1
| WarningCount int `json:"warning_count" console:"header:Warnings"` | ||
| TokenUsage int `json:"token_usage,omitempty" console:"header:Token Usage,format:number,omitempty"` | ||
| EffectiveTokens int `json:"effective_tokens,omitempty" console:"header:Effective Tokens,format:number,omitempty"` | ||
| AmbientContext *AmbientContextMetrics `json:"ambient_context,omitempty"` |
There was a problem hiding this comment.
MetricsData.AmbientContext will render in console output with the autogenerated section title "AmbientContext" (no space) because there is no console tag on this field. To keep the pretty output consistent with other metrics labels, consider adding a console tag like title:Ambient Context (and optionally omitempty) so the section header is user-friendly when rendered via console.RenderStruct.
| AmbientContext *AmbientContextMetrics `json:"ambient_context,omitempty"` | |
| AmbientContext *AmbientContextMetrics `json:"ambient_context,omitempty" console:"title:Ambient Context,omitempty"` |
🧪 Test Quality Sentinel ReportTest Quality Score: 75/100
Test Classification DetailsView All Test Classifications (4 tests)
Flagged Tests — Review Suggested
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 75/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 4 new tests verify behavioral contracts with descriptive assertions and proper build tags. See the comment above for suggestions to improve edge-case coverage.
Generated by the Design Decision Gate workflow for PR #27137. Captures the decision to track first-LLM-invocation token footprint as a proxy for ambient context size in audit and logs outputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (253 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References:
|
|
@copilot Review all |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/72345169-bbae-44e3-a5a4-b6c784b5b3c3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — I reviewed and addressed the outstanding review feedback. Added the console tag so Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
make agent-finish) and capture results (fails in existing external live GitHub API JS test due DNS monitoring proxy)