Skip to content

Complete fidelity-aware command contracts#127

Open
willwashburn wants to merge 1 commit intomainfrom
feat/complete-issue-41
Open

Complete fidelity-aware command contracts#127
willwashburn wants to merge 1 commit intomainfrom
feat/complete-issue-41

Conversation

@willwashburn
Copy link
Copy Markdown
Member

@willwashburn willwashburn commented Apr 26, 2026

Summary

  • populate Codex and OpenCode TurnRecord.fidelity with coverage that distinguishes omitted fields from reported zeroes
  • make compare, summary, waste, plans, and limits consume fidelity/coverage without rendering unknowns as zero
  • add tests for reader fidelity, compare filtering, waste unsupported prerequisites, and partial-fidelity plan confidence

Closes #41

Tests

  • node --test --test-reporter=spec packages/reader/dist/codex.test.js packages/reader/dist/opencode.test.js packages/analyze/dist/compare.test.js packages/analyze/dist/plan-usage.test.js packages/cli/dist/commands/waste.test.js packages/cli/dist/commands/limits.test.js
  • pnpm run test:ts

Open in Devin Review

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

const c = row.usageCoverage[field];
const value = usageValue(row, field);
if (c.knownTurns === 0 && c.unknownTurns === 0 && c.missingTurns > 0) return '—';
const suffix = c.knownTurns === row.turns && c.unknownTurns === 0 ? '' : '*';
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.

🟡 formatUsageCell asterisks every usage number for pre-#41 turns, creating a noisy display regression

For existing users whose ledger consists entirely of pre-#41 turns (no fidelity field), formatUsageCell marks every usage column with * and hasPartialDisplayCoverage triggers the footnote * partial coverage: some numeric fields or prices are unknown…. This happens because updateField at packages/cli/src/commands/summary.ts:417-419 increments unknownTurns for every turn without fidelity. Then the suffix condition at line 429 (c.knownTurns === row.turns && c.unknownTurns === 0) is always false when unknownTurns > 0, producing * on every cell.

This is inconsistent with the rest of the codebase's convention to treat unknown-fidelity turns as "best-effort full" — compare.ts:219-221 includes them, plan-usage.ts:163-164 costs them, waste.ts:159 skips them in fidelity checks. But burn summary penalizes them visually. The cost column is unaffected (since hasCostCoverage returns true for them), making the inconsistency jarring: clean cost but asterisked usage numbers.

Prompt for agents
In formatUsageCell (packages/cli/src/commands/summary.ts:425-431), the suffix condition on line 429 treats unknownTurns the same as missingTurns, but everywhere else in the codebase turns without fidelity are treated as best-effort full. The fix should count unknownTurns alongside knownTurns for the clean-display condition, e.g. by changing the suffix condition to only show asterisk when missingTurns > 0. Similarly, hasPartialDisplayCoverage at line 454-458 should not flag unknownTurns as partial. This ensures pre-#41 users don't see noisy asterisks on every field while still flagging fields that are explicitly declared as missing.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@willwashburn
Copy link
Copy Markdown
Member Author

Recommending close in favor of the granular issue-by-issue PRs (#115, #116, #132, #133, #134, #135), which together cover everything in this PR's scope except the burn summary fidelity surfacing block (~119 lines in packages/cli/src/commands/summary.ts).

That piece is now tracked separately as #136 so it doesn't get lost.

Note on JSON contracts: the granular PRs (#134 plans, #135 compare) use the shared summarizeFidelity / FidelitySummary shape from @relayburn/analyze. This PR invents bespoke shapes (e.g. { matchedTurns, costedTurns, skippedTurns, partialTurns, unknownTurns } for plans, { totalTurns, includedTurns, excludedByClass, … } for compare). The granular shape is more consistent across commands; reviewers should prefer it when porting.

Granular coverage map:

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.

Coverage and fidelity metadata: distinguish missing, zero, aggregate-only, and partial usage

1 participant