Skip to content

Commit 0d5a45f

Browse files
committed
fix(estimate): count every attempt, and price runtime by prompt size
Review follow-ups. Both blocking items understated cost in the same direction, and both are fixed from data the run records already carry. Output tokens counted only the final attempt while prompt tokens were multiplied by the attempt count, so a retried pass recorded prompt x2 and output x1. Because calibrate divides output by a request count that includes retries, every retry dragged outputTokensPerRequest down and estimate then understated output for everyone. runAuditWithAutoFix now reports promptChars and outputChars accumulated across attempts, so both sides are exact rather than the final attempt scaled up: the retry's prompt carries a correction section the x2 approximation missed, and its discarded first response cost tokens to generate. Characters rather than tokens keeps the approximation in one place, and keeps whole prompts from being retained on the result. Runtime was requests x a flat msPerRequest, which is prompt-size-blind — the exact case the command is pitched at. Calibration now carries a fixed msPerRequest plus a marginal msPerPromptToken, least-squares fitted across records when they differ in prompt size. When they cannot separate the terms — one record, or every run the same size — the measured average is split using the proportion the defaults imply, so the magnitude stays measured even where the shape is assumed. A negative fitted term falls back the same way. The defaults still sum to the previous 45s at a 2,500-token prompt. Also: - The --clone warning selected on the post-applies_to count, so a repo that was checked out but matched no files was told to clone what it already had. RepoEstimate carries the pre-scoping count and the two cases now get separate warnings; the second is the more useful, since it means a pack is pointed at a repo it cannot see. - Docs and --help claimed the command "mutates nothing" while documenting --clone. Reworded, and the runtime model is documented. - formatDuration handed over to minutes at 90s while rounding minutes from 60s, so "1 minute(s)" was unreachable: 89s rendered as seconds and 90s jumped to 2 minutes. - runEstimate printed targetErrors to stderr even when the report went to stdout, where the caveats already list them. Now only when --output redirects the report to a file. The 0 exit is left as it was, with a comment marking it deliberate rather than an oversight. Left alone: the unguarded readFileSync(configPath). It matches runRun, so fixing one without the other would just make the two inconsistent. Breaking change, from the original commit rather than this one: PackOutcome.usage is required and PackOutcome is exported from source/index.ts, so an external consumer constructing one breaks at compile time.
1 parent fa8bfa6 commit 0d5a45f

8 files changed

Lines changed: 406 additions & 39 deletions

File tree

docs/cli/index.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ The same validator, dedup logic, and findings model apply in both contexts, so w
6060

6161
## `estimate`
6262

63-
Sizes an audit **before** it runs: how many repositories and rule packs are in scope, how many files they put in front of the model, and roughly how many model requests, tokens, and minutes that costs. It runs no model, files nothing, and mutates nothing — useful when you are about to point Sentinel at a dozen more repositories, or adding a pack to every target and want to know what that does to the nightly window.
63+
Sizes an audit **before** it runs: how many repositories and rule packs are in scope, how many files they put in front of the model, and roughly how many model requests, tokens, and minutes that costs. It runs no model and files no issues — `--clone` is the one flag that writes anything, checking out missing repos. Useful when you are about to point Sentinel at a dozen more repositories, or adding a pack to every target and want to know what that does to the nightly window.
6464

6565
```bash
6666
npx @nanocollective/sentinel estimate
@@ -90,8 +90,10 @@ Calibrated from the last 6 run record(s).
9090

9191
### How the figures are produced
9292

93-
The token figure is **measured, not guessed**: `estimate` assembles the same prompts the audit would send — the pack body, the reporting contract, and the source files scoped by each pack's `applies_to.paths` — and counts them. What varies between installs is the per-request cost, so the request, token, and runtime figures are calibrated from the run records the last ten runs committed. Every run is instrumented for this: it records how long each pack pass took, how many model requests it made (auto-fix retries included), and the tokens it sent and received.
93+
The token figure is **measured, not guessed**: `estimate` assembles the same prompts the audit would send — the pack body, the reporting contract, and the source files scoped by each pack's `applies_to.paths` — and counts them. What varies between installs is the per-request cost, so the request, token, and runtime figures are calibrated from the run records the last ten runs committed. Every run is instrumented for this: it records how long each pack pass took, how many model requests it made (auto-fix retries included), and the tokens it sent and received across every attempt — an auto-fix retry resends the prompt and generates a second response, and both are counted.
94+
95+
Runtime is **not** a flat per-request average. A request costs a fixed amount regardless of size plus an amount that tracks prompt size, and both terms are fitted from the records, so sizing a config far larger than anything you have run is not priced as though the prompts stayed the same. When the records cannot separate the two — a single run, or every run the same size — the measured average is split using the proportion the built-in defaults imply.
9496

9597
Until a run has been recorded, the figures fall back to built-in defaults, and the output says so. Treat a first, uncalibrated estimate as an order of magnitude rather than a number to schedule against.
9698

97-
Repositories already checked out under `--workspace` are measured from their real files. Any that are not are counted with zero files and called out in the output, so a partial estimate never reads as the whole picture — pass `--clone` to check the rest out first.
99+
Repositories already checked out under `--workspace` are measured from their real files. Any that are not are counted with zero files and called out in the output, so a partial estimate never reads as the whole picture — pass `--clone` to check the rest out first. A repo that *is* checked out but whose files no pack matches gets a separate warning: nothing needs cloning, but a pack is pointed at a repository it cannot see.

source/cli.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -338,8 +338,8 @@ async function runRun(argv: string[]): Promise<number> {
338338
const ESTIMATE_USAGE = `sentinel estimate [options]
339339
340340
Size an audit before running it: repositories, rule packs, files, model
341-
requests, tokens, and wall-clock runtime. Runs no model, files nothing, and
342-
mutates nothing.
341+
requests, tokens, and wall-clock runtime. Runs no model and files no issues;
342+
--clone is the one flag that writes anything, checking out missing repos.
343343
344344
Figures are calibrated from the committed run records when any exist, so they
345345
sharpen against your own hardware and model. Repos already checked out under
@@ -386,10 +386,17 @@ async function runEstimate(argv: string[]): Promise<number> {
386386
},
387387
);
388388

389-
writeReport(renderEstimate(estimate), flagStr(flags, 'output'));
390-
for (const error of estimate.targetErrors) {
391-
console.error(`target: ${error}`);
389+
const output = flagStr(flags, 'output');
390+
writeReport(renderEstimate(estimate), output);
391+
// The estimate already renders these as caveats, so repeat them on stderr
392+
// only when the report went to a file and nobody would otherwise see them.
393+
if (output) {
394+
for (const error of estimate.targetErrors) {
395+
console.error(`target: ${error}`);
396+
}
392397
}
398+
// Deliberately 0 even when targets failed: estimate is advisory, and a
399+
// partial estimate is still useful — the caveats say what is missing.
393400
return 0;
394401
}
395402

source/orchestrator/auto-fix.spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,44 @@ test('buildAutoFixPrompt renders document-level errors without an index', t => {
129129
t.true(prompt.includes('- document: no array found'));
130130
t.false(prompt.includes('finding[-1]'));
131131
});
132+
133+
// The audit layer turns these into the prompt/output token figures `estimate`
134+
// calibrates from, so they have to cover every attempt rather than the last.
135+
test('reports the characters sent and received on a single attempt', async t => {
136+
const output = JSON.stringify([GOOD]);
137+
const runner = queuedRunner([{ok: true, output}]);
138+
const result = await runAuditWithAutoFix('prompt', MODEL, runner);
139+
t.is(result.promptChars, 'prompt'.length);
140+
t.is(result.outputChars, output.length);
141+
});
142+
143+
test('accumulates characters across a retry', async t => {
144+
const first = JSON.stringify([BAD]);
145+
const second = JSON.stringify([GOOD]);
146+
const runner = queuedRunner([
147+
{ok: true, output: first},
148+
{ok: true, output: second},
149+
]);
150+
const result = await runAuditWithAutoFix('prompt', MODEL, runner);
151+
152+
t.is(result.attempts, 2);
153+
// Both responses counted, not just the one that validated.
154+
t.is(result.outputChars, first.length + second.length);
155+
// Both prompts counted, and the retry carried the correction section, so
156+
// the total exceeds twice the original rather than matching it.
157+
t.is(
158+
result.promptChars,
159+
(runner.prompts[0]?.length ?? 0) + (runner.prompts[1]?.length ?? 0),
160+
);
161+
t.true(result.promptChars > 'prompt'.length * 2);
162+
});
163+
164+
test('a process failure is not retried and counts one exchange', async t => {
165+
const runner = queuedRunner([
166+
{ok: false, output: '', error: 'nanocoder missing'},
167+
]);
168+
const result = await runAuditWithAutoFix('prompt', MODEL, runner);
169+
t.is(result.attempts, 1);
170+
t.is(result.promptChars, 'prompt'.length);
171+
t.is(result.outputChars, 0);
172+
});

source/orchestrator/auto-fix.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ export interface AutoFixOptions extends RunnerOptions {
2222
export interface AutoFixResult extends AuditResult {
2323
/** Number of model runs performed (1 = succeeded or failed on first try). */
2424
attempts: number;
25+
/**
26+
* Characters sent across every attempt. A retry resends the audit prompt plus
27+
* a correction section, so this is the real total rather than a multiple of
28+
* the first prompt. Carried as characters, not tokens, to keep the token
29+
* approximation in one place ({@link ../run/estimate.js estimateTokens}).
30+
*/
31+
promptChars: number;
32+
/** Characters returned across every attempt, the discarded ones included. */
33+
outputChars: number;
2534
}
2635

2736
function formatErrors(errors: ValidationError[]): string {
@@ -83,12 +92,16 @@ export async function runAuditWithAutoFix(
8392

8493
let result = await runAudit(prompt, model, runner, options);
8594
let attempts = 1;
95+
let promptChars = prompt.length;
96+
let outputChars = result.raw.length;
8697

8798
while (!result.ok && !result.runError && attempts < maxAttempts) {
8899
const fixPrompt = buildAutoFixPrompt(prompt, result);
89100
result = await runAudit(fixPrompt, model, runner, options);
90101
attempts++;
102+
promptChars += fixPrompt.length;
103+
outputChars += result.raw.length;
91104
}
92105

93-
return {...result, attempts};
106+
return {...result, attempts, promptChars, outputChars};
94107
}

source/run/audit.spec.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type {ModelConfig} from '../config/types.js';
33
import type {ModelRunner, ModelRunResult} from '../orchestrator/types.js';
44
import type {RulePack} from '../rule-packs/types.js';
55
import {auditPack} from './audit.js';
6+
import {estimateTokens} from './estimate.js';
67

78
console.log('\nrun/audit.spec.ts');
89

@@ -70,22 +71,44 @@ test('measures the pass so estimates have something to calibrate on', async t =>
7071
t.true(outcome.usage.outputTokens > 0);
7172
});
7273

73-
test('counts the prompt once per attempt', async t => {
74-
// Malformed output the auto-fix loop retries, so two attempts are made.
75-
const single = await auditPack(
74+
test('counts every attempt on both the prompt and the output side', async t => {
75+
// The same malformed output, run once and then with the retry allowed, so
76+
// the only difference between the two is the second attempt.
77+
const context = {repoName: 'org/a', files: []};
78+
const once = await auditPack(
7679
PACK,
77-
{repoName: 'org/a', files: []},
80+
context,
7881
MODEL,
79-
runner({ok: true, output: JSON.stringify([FINDING])}),
82+
runner({ok: true, output: 'not json'}),
83+
{maxAttempts: 1},
8084
);
8185
const retried = await auditPack(
8286
PACK,
83-
{repoName: 'org/a', files: []},
87+
context,
8488
MODEL,
8589
runner({ok: true, output: 'not json'}),
8690
);
91+
92+
t.is(once.attempts, 1);
8793
t.is(retried.attempts, 2);
88-
t.is(retried.usage.promptTokens, single.usage.promptTokens * 2);
94+
// The discarded first response still cost tokens to generate. Counting only
95+
// the final attempt understated output, and since calibration divides by a
96+
// request count that includes retries, it dragged every estimate down.
97+
t.is(retried.usage.outputTokens, once.usage.outputTokens * 2);
98+
// The retry resends the prompt plus a correction section, so the prompt side
99+
// is more than twice the first attempt rather than exactly twice.
100+
t.true(retried.usage.promptTokens > once.usage.promptTokens * 2);
101+
});
102+
103+
test('a pass that never retries counts one prompt and one response', async t => {
104+
const outcome = await auditPack(
105+
PACK,
106+
{repoName: 'org/a', files: []},
107+
MODEL,
108+
runner({ok: true, output: JSON.stringify([FINDING])}),
109+
);
110+
t.is(outcome.attempts, 1);
111+
t.is(outcome.usage.outputTokens, estimateTokens(JSON.stringify([FINDING])));
89112
});
90113

91114
test('surfaces a run error in the outcome', async t => {

source/run/audit.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import type {ModelRunner} from '../orchestrator/types.js';
1414
import {buildAuditPrompt} from '../prompt/build.js';
1515
import type {SourceFile} from '../prompt/types.js';
1616
import type {RulePack} from '../rule-packs/types.js';
17-
import {estimateTokens} from './estimate.js';
17+
import {tokensFromChars} from './estimate.js';
1818
import type {PackOutcome} from './types.js';
1919

2020
/** The repository material one pack pass audits. */
@@ -56,10 +56,12 @@ export async function auditPack(
5656
raw: result.raw,
5757
usage: {
5858
durationMs,
59-
// A retry resends the audit prompt; the correction preamble is small
60-
// beside it, so attempts x the base prompt is a fair figure.
61-
promptTokens: estimateTokens(prompt) * result.attempts,
62-
outputTokens: estimateTokens(result.raw),
59+
// Both sides are the real totals across every attempt, not the final
60+
// one scaled up: a retry resends the prompt plus a correction section,
61+
// and the output it discards still cost tokens to generate. Counting
62+
// them keeps calibration honest, since requests includes retries too.
63+
promptTokens: tokensFromChars(result.promptChars),
64+
outputTokens: tokensFromChars(result.outputChars),
6365
},
6466
};
6567
}

0 commit comments

Comments
 (0)