Skip to content

fix(report): follow the ReportV1 schema where tskl.es prices value, and mine commit history - #521

Merged
theCodeDrift merged 4 commits into
mainfrom
fix/report-without-review-history
Oct 16, 2026
Merged

theCodeDrift merged 4 commits into
mainfrom
fix/report-without-review-history

Conversation

@theCodeDrift

@theCodeDrift theCodeDrift commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Testing onboarding on a C#/.NET repository produced a report that priced every rule at $0 next to 114 findings in the code today. Value was estimated from review comments alone, using inputs the agent guessed, and that repository reviews before its pull requests open. tskl.es has since taken over all value estimation, and its live ReportV1 schema changed to match. This PR brings the CLI and its recipes in line.

Every current nightly fails to publish right now. The live schema removed projection and rejects any field it doesn't list, so a nightly that still sends projection gets a 400. report is in no release (0.12.0 predates it), so only nightlies are affected.

Report payload (follows the live schema)

  • The vendored ReportV1 is regenerated from https://tskl.es/api/v1/schema.
  • The payload carries no projection, and no rule carries complexity. tskl.es estimates reviewer time, agent tokens, and dollars itself, from findings, each rule's surfaced, and each candidate's estimate.
  • Cloud candidates carry estimatedViolations, the agent's estimate of how many instances of the pattern are in the code today. It is required in the facts and never appears on a written rule, which carries the measured violations instead.
  • The facts file no longer accepts estimates (contextTokens, callsPerCatch) or a candidate's complexity; supplying either is refused. The draft drops its default inputs, and missing no longer lists estimates.

Recipes

  • Onboard topic v8: names commit history as a mining source, read for fixes that recur. In the tested repository that is where the real signal was. Commit evidence helps choose rules and never counts toward surfaced. The agent also notes how many instances of each runtime candidate it saw, for the report's estimate.
  • Report topic v3: only review comments count toward surfaced. The agent supplies each candidate's estimatedViolations and no pricing input. When restating numbers, it keeps measured counts, review counts, and its own estimates apart.

OpenSpec

Two changes, both archived here:

  • report-without-review-history: the commit-history source and the review-only rule for surfaced.
  • report-server-prices-value: the schema follow-through. It deliberately drops three scenarios: the small-projection gate, an all-zero explanation the first change added (superseded before it ever reached main), and the inputs-only payload. OpenSpec has no way to drop a single scenario, so two requirements are replaced under new titles, with the reason recorded. A trial archive confirmed only those three scenarios are dropped.

Checks

pnpm typecheck, pnpm lint, and pnpm test (2,336 tests) all pass locally. The assembled payload validates against the regenerated schema, which matches the live one.

Refs #520

@theCodeDrift theCodeDrift changed the title fix(recipes): mine commit history and explain an all-zero report estimate fix(report): follow the ReportV1 schema where tskl.es prices value, and mine commit history Oct 16, 2026
…mate

Onboard topic v8 names commit history as a source and keeps its evidence
out of surfaced. Report topic v3 counts review comments only and has the
agent explain a $0 estimate when review raised none of the patterns.

Refs #520
The payload drops projection and each rule's complexity; Cloud
candidates carry the agent's estimatedViolations. The facts file stops
accepting estimates, and the report recipe stops asking for them and
drops the all-zero explanation the server's pricing makes wrong.

Refs #520
@theCodeDrift
theCodeDrift force-pushed the fix/report-without-review-history branch from f708e52 to 27960b2 Compare October 16, 2026 06:50
@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 2m 39s —— View job


Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no threads, reviews, or comments beyond the trigger comment itself, so this is the first review of this PR; there is nothing previously-raised to classify as addressed or still-open.

  • Read prior review history
  • Fetch PR diff and metadata
  • Review schema/type changes (report-v1.schema.json, types.ts, report-facts.ts)
  • Review assemble.ts / draft.ts logic changes
  • Review recipe changes (onboard.md, report.md)
  • Review OpenSpec deltas for the "replace, not patch" rule
  • Review test coverage changes
  • Post final review summary

[New] No findings. The diff is internally consistent and well-covered:

  • complexity/projection/estimates are removed consistently across types.ts, assemble.ts, draft.ts, report-facts.ts, and the regenerated report-v1.schema.json — no stale reference to any of them remains anywhere in the diff (recipes, specs, or tests).
  • estimatedViolations is required in cloudCandidateFacts (report-facts.ts), carried through 1:1 in mergeRules (assemble.ts), typed as optional on ReportRule (correct, since only candidates carry it), and covered by tests for both the missing-field rejection and the written-rule-never-carries-it invariant (report.test.ts).
  • The OpenSpec deltas get the tricky part right: this repo's CLAUDE.md specifically warns that a MODIFIED Requirements block silently drops anything it omits, and that renaming a requirement's title makes a MODIFIED block a no-op. report-server-prices-value's delta avoids both failure modes by using REMOVED Requirements + ADDED Requirements pairs (with Reason/Migration notes) for the three requirements it restates under new titles ("Publishing is gated on a meaningful report" → "...on three working Taskless rules", "The report recipe drives the flow" → "...drives publishing and deletion", "The payload carries projection inputs and no rates" → "...carries no value inputs"), rather than a lossy MODIFIED. The merged openspec/specs/cli-report/spec.md matches what the deltas produce, and the only scenarios actually dropped are the three the PR body names (small-projection gate, the all-zero explanation, the inputs-only payload scenario).
  • One changeset, grown rather than duplicated, matches the "one changeset at the bottom of the stack" guidance, and the patch bump is correct pre-1.0 for new surface (estimatedViolations, commit-history mining).
  • Recipe text (onboard.md v8, report.md v3) and their specs stay in lockstep with the schema change — report.md's step 2 and step 4 wording line up exactly with what onboard.test.ts/report.test.ts assert against.

Nothing to flag; CI will confirm typecheck/lint/test as reported in the PR description.
· Branch

@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @github-actions — "Claude finished @theCodeDrift's task in 2m 39s… Review complete"
#521 (comment)

Thanks. No findings to address; the review confirms the field removals are consistent and that the OpenSpec deltas drop only the three scenarios the PR body names.

— AI Coding Agent

@theCodeDrift
theCodeDrift merged commit 826cf88 into main Oct 16, 2026
4 checks passed
@theCodeDrift
theCodeDrift deleted the fix/report-without-review-history branch October 16, 2026 08:51
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.

1 participant