mirror of https://github.com/garrytan/gstack.git
fix(analytics): avg duration handles both quoted and unquoted duration_s
The per-skill (avg X) column in gstack-analytics silently rendered as "0s" for any /skill whose completion events emit "duration_s" as a quoted JSON string. The completion-status preamble that every skill runs (scripts/resolvers/preamble/generate-completion-status.ts:70) and gstack-codex-probe both emit "duration_s":"<N>", so the dominant case was wrong; gstack-telemetry-log emits the unquoted form which is why the bug didn't show up everywhere. The AVG_DUR awk block split each line on the literal "duration_s": prefix and then ran gsub(/[^0-9.].*/, "", parts[2]). For unquoted 600,"ts":... the gsub matches at the comma and yields "600". For quoted "600","ts":... it matches at the leading double-quote (position 0) and the trailing .* consumes the entire rest of the line, leaving an empty string that never accumulates. The companion TOTAL_DURATION calculation (line 114) uses a different parser keyed on -F'[:,]' field splitting + non-greedy gsub, so it isolates the value before stripping and was always correct. Only the per-skill average was broken. Fix: strip an optional leading double-quote with sub(/^"/, ...) before the existing gsub so quoted and unquoted forms take the same path. Adds test/gstack-analytics-avg-duration.test.ts covering quoted-only, unquoted-only, mixed, and total-time aggregation. The test fails on the pre-fix tree (expected "2m", got "0s") and passes after the fix.
This commit is contained in:
parent
cf50443b63
commit
2cc9f6477e
|
|
@ -168,7 +168,9 @@ echo "$SKILL_COUNTS" | while read -r COUNT SKILL; do
|
||||||
# Extract duration_s value using split on "duration_s":
|
# Extract duration_s value using split on "duration_s":
|
||||||
n = split($0, parts, "\"duration_s\":")
|
n = split($0, parts, "\"duration_s\":")
|
||||||
if (n >= 2) {
|
if (n >= 2) {
|
||||||
# parts[2] starts with the value, e.g. "142,"
|
# parts[2] starts with the value: unquoted (142,...) or quoted ("142",...).
|
||||||
|
# Strip a leading double-quote so the digit-extraction below handles both forms.
|
||||||
|
sub(/^"/, "", parts[2])
|
||||||
gsub(/[^0-9.].*/, "", parts[2])
|
gsub(/[^0-9.].*/, "", parts[2])
|
||||||
if (parts[2]+0 > 0) { total += parts[2]; count++ }
|
if (parts[2]+0 > 0) { total += parts[2]; count++ }
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,87 @@
|
||||||
|
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||||
|
import { spawnSync } from 'child_process';
|
||||||
|
import * as fs from 'fs';
|
||||||
|
import * as os from 'os';
|
||||||
|
import * as path from 'path';
|
||||||
|
|
||||||
|
const ROOT = path.resolve(import.meta.dir, '..');
|
||||||
|
const BIN = path.join(ROOT, 'bin', 'gstack-analytics');
|
||||||
|
|
||||||
|
// Verifies the per-skill avg-duration calculation in the bash dashboard
|
||||||
|
// (bin/gstack-analytics) parses both unquoted and quoted "duration_s"
|
||||||
|
// values from skill-usage.jsonl. The completion-status preamble (and
|
||||||
|
// gstack-codex-probe) emit quoted strings; gstack-telemetry-log emits
|
||||||
|
// unquoted numbers. The previous awk regex stripped from the leading
|
||||||
|
// quote, silently yielding avg=0 for quoted-only skills.
|
||||||
|
describe('gstack-analytics avg duration parsing', () => {
|
||||||
|
let stateDir: string;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-analytics-test-'));
|
||||||
|
fs.mkdirSync(path.join(stateDir, 'analytics'), { recursive: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
function run(jsonl: string[]): string {
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(stateDir, 'analytics', 'skill-usage.jsonl'),
|
||||||
|
jsonl.join('\n') + '\n',
|
||||||
|
);
|
||||||
|
const r = spawnSync('bash', [BIN, 'all'], {
|
||||||
|
env: { ...process.env, GSTACK_STATE_DIR: stateDir },
|
||||||
|
encoding: 'utf-8',
|
||||||
|
});
|
||||||
|
if (r.status !== 0) {
|
||||||
|
throw new Error(`gstack-analytics failed (status ${r.status}): ${r.stderr}`);
|
||||||
|
}
|
||||||
|
return r.stdout;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Pull "(avg <value>)" out of a /skill row.
|
||||||
|
function avgFor(report: string, skill: string): string {
|
||||||
|
const re = new RegExp(`/${skill}\\b[^\\n]*\\(avg ([^)]+)\\)`);
|
||||||
|
const m = report.match(re);
|
||||||
|
if (!m) throw new Error(`no avg row for /${skill} in:\n${report}`);
|
||||||
|
return m[1];
|
||||||
|
}
|
||||||
|
|
||||||
|
test('quoted "duration_s" values are averaged (regression: previously read as 0)', () => {
|
||||||
|
const output = run([
|
||||||
|
'{"skill":"review","duration_s":"120","outcome":"success","ts":"2026-05-25T10:00:00Z"}',
|
||||||
|
'{"skill":"review","duration_s":"180","outcome":"success","ts":"2026-05-25T11:00:00Z"}',
|
||||||
|
'{"skill":"review","duration_s":"60","outcome":"success","ts":"2026-05-25T12:00:00Z"}',
|
||||||
|
]);
|
||||||
|
// (120+180+60)/3 = 120s = 2m
|
||||||
|
expect(avgFor(output, 'review')).toBe('2m');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('unquoted numeric "duration_s" values are averaged', () => {
|
||||||
|
const output = run([
|
||||||
|
'{"skill":"qa","duration_s":300,"outcome":"success","ts":"2026-05-25T13:00:00Z"}',
|
||||||
|
'{"skill":"qa","duration_s":420,"outcome":"success","ts":"2026-05-25T14:00:00Z"}',
|
||||||
|
]);
|
||||||
|
// (300+420)/2 = 360s = 6m
|
||||||
|
expect(avgFor(output, 'qa')).toBe('6m');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('mixed quoted and unquoted values average together', () => {
|
||||||
|
const output = run([
|
||||||
|
'{"skill":"ship","duration_s":"30","outcome":"success","ts":"2026-05-25T10:00:00Z"}',
|
||||||
|
'{"skill":"ship","duration_s":50,"outcome":"success","ts":"2026-05-25T11:00:00Z"}',
|
||||||
|
]);
|
||||||
|
// (30+50)/2 = 40s — under 60 so shown in seconds, exercising both paths.
|
||||||
|
expect(avgFor(output, 'ship')).toBe('40s');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('total time sums quoted and unquoted across all skills', () => {
|
||||||
|
const output = run([
|
||||||
|
'{"skill":"review","duration_s":"120","outcome":"success","ts":"2026-05-25T10:00:00Z"}',
|
||||||
|
'{"skill":"qa","duration_s":300,"outcome":"success","ts":"2026-05-25T13:00:00Z"}',
|
||||||
|
]);
|
||||||
|
// 120+300 = 420s = 7m
|
||||||
|
expect(output).toContain('Total time: 7m');
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in New Issue