Skip to content

chore(biome): enforce block statements - #679

Merged
goosewobbler merged 2 commits into
mainfrom
chore/biome-block-statements
Oct 22, 2026
Merged

goosewobbler merged 2 commits into
mainfrom
chore/biome-block-statements

Conversation

@goosewobbler

@goosewobbler goosewobbler commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Description

Implements #666: every if/else/loop body is a block, enforced by Biome.

Two commits, kept apart so the mechanical one is easy to skip in review:

  1. chore(biome): enforce block statements — biome.jsonc turns style/useBlockStatements from "off" to:

    "useBlockStatements": { "level": "error", "fix": "safe" }

    Marking the fix safe means the pre-commit biome check --write (lint-staged) adds the braces itself instead of only reporting them, so contributors and agents don't have to.

  2. style: add braces to every single-statement body — the sweep: biome lint --write --only=style/useBlockStatements . then biome format --write .. 703 sites in 134 files (src, tests and scripts/). Nothing else changes: with whitespace and {/} stripped, every one of the 134 files is identical to main.

eslint.config.js has no curly rule, so nothing conflicts with it.

Merging this before other open PRs

The sweep touches most source files, so open branches will conflict after it lands. A branch resolves cleanly by running the same fix before merging main, so both sides already carry identical braces and only real changes conflict:

pnpm exec biome lint --write --only=style/useBlockStatements . && pnpm exec biome format --write .
git commit -am "style: add braces to every single-statement body"
git merge origin/main

I'll do that on the PRs I'm driving (#616, #668, #673, #674) once this merges.

Follow-up: .git-blame-ignore-revs

The sweep should be listed in .git-blame-ignore-revs so git blame skips it, but the entry has to be the commit that lands on main — with a squash merge that SHA only exists after merging. I'll add the file with the merged SHA as a one-line follow-up.

Related Issue(s)

Closes #666

Type of Change

  • feat — New feature
  • fix — Bug fix
  • docs — Documentation change
  • refactor — Code refactoring
  • test — Adding or updating tests
  • chore — Maintenance, dependencies, tooling
  • Breaking change — Requires major version bump

Scope

  • @releasekit/version
  • @releasekit/notes
  • @releasekit/publish
  • @releasekit/config
  • @releasekit/core
  • CI / workflows
  • Root / monorepo

Checklist

  • Lint passes (pnpm lint)
  • Tests pass (pnpm test)
  • Typecheck passes (pnpm typecheck)
  • Documentation updated (if applicable)
  • Breaking changes noted in description

Testing Notes

  • After the sweep, biome lint --only=style/useBlockStatements . reports nothing; before it, 703 errors.
  • Equivalence check: for each of the 134 changed files, the main version and this branch's version are identical once whitespace and braces are removed.
  • pnpm build, pnpm lint (29 warnings, same as main), pnpm typecheck pass.
  • pnpm test:coverage (what CI runs): 15/15 tasks pass with thresholds enforced. pnpm --filter @releasekit/integration-tests test: 63 passed.

Generated by Claude Code

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge.

Summary

Enables useBlockStatements as an error in biome.jsonc and marks its fix safe. Adds braces across source files, tests, and scripts.

  • Control statements use braces throughout the repository.

Reviews (1) · Last reviewed commit: "style: add braces to every single-statem..." · Reviewed by Greptile

Turns on style/useBlockStatements as an error and marks its fix safe, so
the pre-commit `biome check --write` adds the braces instead of only
reporting them. Closes #666.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
Mechanical sweep for the new useBlockStatements rule:
`biome lint --write --only=style/useBlockStatements .` then
`biome format --write .`. 703 sites in 134 files; with whitespace and
braces stripped, every file is identical to before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler pushed a commit that referenced this pull request Oct 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Standing release PR: #614 · v0.42.0 queued (4 packages) · open 82d 14m · ✅ ready to merge

Release Preview — v0.41.2 → v0.41.3

Note: Labels on this PR are advisory in standing-pr mode. Bumps come from conventional commits in the standing PR; override by editing labels on the standing PR itself. Add release:immediate to bypass the standing PR and release this PR directly.

These changes will be added to the release PR (#614) when merged:

Changelog

monorepo 0.41.2 → 0.41.3

Changed

  • add braces to every single-statement body
  • enforce block statements (biome) (#666)

All packages → 0.41.3 (sync versioning)

  • @releasekit/version
  • @releasekit/notes
  • @releasekit/publish
  • @releasekit/release

After merge — predicted release

No version escalation — this PR's changes will be included in the queued release without affecting the projected versions.

Package Standing PR This PR After merge
monorepo 0.42.0 0.41.3 0.42.0

Updated automatically by ReleaseKit

@goosewobbler
goosewobbler merged commit 87bc321 into main Oct 22, 2026
34 checks passed
@goosewobbler
goosewobbler deleted the chore/biome-block-statements branch October 22, 2026 18:29
@github-actions github-actions Bot mentioned this pull request Oct 22, 2026
goosewobbler pushed a commit that referenced this pull request Oct 22, 2026
Same sweep as #679, run on this branch so merging main resolves cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler pushed a commit that referenced this pull request Oct 22, 2026
Same sweep as #679, run on this branch so merging main resolves cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler pushed a commit that referenced this pull request Oct 22, 2026
Same sweep as #679, run on this branch so merging main resolves cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler pushed a commit that referenced this pull request Oct 22, 2026
main now enforces style/useBlockStatements (#679).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler pushed a commit that referenced this pull request Oct 22, 2026
main now enforces style/useBlockStatements (#679).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler added a commit that referenced this pull request Oct 22, 2026
* docs: add commands and gotchas to AGENTS.md, and drop CLAUDE.md

AGENTS.md keeps the architecture invariants and gains what an agent can't
infer from the tree: the checks to run before pushing (there's no pre-push
hook), why a one-package run can test stale dist/, that a package's own lint
script skips ESLint, which suites only CI runs, and three gotchas: Turborepo's
strict env mode drops opt-in variables such as RELEASEKIT_NOTES_E2E, Vitest
hides output when it detects an agent, and Biome finds no files in a worktree
under .claude/. It also lists the conventions the linters don't check.

Claude Code reads AGENTS.md itself when there's no CLAUDE.md, and the
claude-code-action pin installs a version that does, so the CLAUDE.md that
only imported AGENTS.md goes.

CONTRIBUTING.md asked for Node 18, while the packages need 22, and said
there's no ESLint. It also claimed `pnpm --filter ... build` builds a
package's dependencies, linked a BOOTSTRAP.md that no longer exists, and
showed a package layout most packages don't match. The PR template and the
commit scopes gain @releasekit/release, and the bug report's Node placeholder
moves to a supported version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(agents): point the braces rule at #666

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(agents): say which suites pnpm test skips

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(agents): drop the braces rule now that Biome enforces it (#679)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(contributing): require pnpm 12 and drop the .nvmrc aside

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(agents): trim commands to what an agent can't infer; drop the any rule

#682 makes noExplicitAny an error, so the rule no longer needs prose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(agents): drop the Vitest version from the reporter gotcha

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

---------

Co-authored-by: Claude <noreply@anthropic.com>
goosewobbler added a commit that referenced this pull request Oct 22, 2026
* chore(biome): flag console in package source

Package code logs through the logger: core's, or version's own
utils/logging.ts. Biome's noConsole now enforces that in packages/*/src,
except the loggers themselves and the CLI command layer (command.ts,
commands/), which owns the terminal: the JSON result on stdout and the
final error message on stderr.

The version package's error paths dumped stack traces and git output with
console.error next to log(..., 'error'); they now go through log, which
writes the error level to stderr in both modes, so the stream is
unchanged. Three outputs that aren't diagnostics keep console behind a
suppression: version's --json result, preview's dry-run comment, and the
RELEASEKIT_DEBUG prompt dump (folded into one write, same bytes), which
must print regardless of log level or --quiet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(version): stop the strategy unit tests running real cargo

versionStrategies.spec.ts mocks node:fs, so the Cargo.lock lookup in
syncCargoLockfile finds a lock and the real sync shells out to
`cargo update --workspace --offline` — 78 times per run. On a cold CI
runner that pushes "should update all packages to the same version" past
its 5s timeout; it failed that way on #668 and #671. Mock the sync like
the other collaborators.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(version): drop the comment on the cargoLock mock

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* style: add braces to every single-statement body

Same sweep as #679, run on this branch so merging main resolves cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* chore(biome): shorten the noConsole override comment and suppression reasons

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* chore(biome): reword the noConsole override and debug-dump suppression

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

---------

Co-authored-by: Claude <noreply@anthropic.com>
goosewobbler added a commit that referenced this pull request Oct 23, 2026
…618)

* test(notes): make the LLM e2e infra-tolerant and close a false green

First half of #589 — the spec change that has to land before the check can block
merges. The workflow job and the OLLAMA_API_KEY secret are still outstanding.

`isProviderUnreachable` splits the two things a real-provider check can fail on. Only
an LLMError the provider positively classified as transient (timeout, 429, 5xx,
socket) counts as infra; a non-retryable 4xx is a malformed request, and so is
anything that isn't an LLMError — assertion failures and TypeErrors included. An
unclassified LLMError fails too: every provider sets the flag explicitly, so an absent
one means something unrecognised happened, and a visible red beats a silent skip. The
predicate is a separate module with its own always-running spec, since it decides
whether the gated check can go red at all.

Found while verifying the skip path: the chunk-boundary test passed against an
unreachable server in 20ms. enhanceAndCategorize never throws on a provider failure —
its per-chunk fallback preserves the input entries — so asserting entry counts holds
just as well with no provider at all. The test could not have detected the thing #589
wants it to guard.

Two changes close that. A beforeAll probe establishes reachability once, so an absent
host skips rather than being inferred from a later assertion; and the chunk test now
requires that some descriptions actually differ from their inputs, which the fallback
never produces. Verified against a dead port: both tests skip, where one previously
reported a false pass.

The positive path is unverified locally (no Ollama here) — it needs a run against a
reachable model before the check is made blocking.

Part of #589.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* test(notes): skip on a mid-run outage instead of reporting a regression

From Greptile review of #618, and correct: the up-front probe cannot see an outage that
starts after it. enhanceAndCategorize swallows retryable provider errors and falls back
to the input entries verbatim, so a host that dies mid-test produces exactly the same
observable as a model that declined to rewrite — and the new rewrite assertion would
have failed the check as a product regression.

"Nothing was rewritten" now re-probes before being called a regression. The provider is
the only thing that can disambiguate, since the fallback is silent by design.

Verified both directions against mock Ollama servers, because a fix here can fail either
way — too eager and every regression becomes a silent skip:

- Answers the probe, then 503s: skips with "SKIPPED mid-run: provider became
  unreachable", where it previously failed red.
- Stays up but returns unparseable output: still fails red ("expected 0 to be greater
  than 0"), which is the regression the check exists to catch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* test(notes): count rewrites per chunk so a partial outage can't pass

From Greptile's re-review of #618, and right: chunks fall back independently, so a host
that dies after the first chunk left ~30 rewrites in place, kept the total above zero,
and sailed past the re-probe into a green check — a quieter version of the false green
this PR set out to remove.

Counting per chunk closes it. Either chunk coming back untouched now triggers the
re-probe, which is the correct reading for a test whose subject is the boundary: the
point is that both sides of it were enhanced, not that 30 of 35 entries were.

Chunk size and entry count now derive from LLM_DEFAULTS rather than a hardcoded 35, so
the case keeps straddling the boundary if that constant moves.

Verified with a mock that answers the probe, serves the first chunk correctly, then
503s: skips, where before the fix it passed. The healthy-but-broken mock still fails red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* test(notes): require transport evidence to skip, and retry like production

Addresses the review findings on #618.

isProviderUnreachable no longer accepts retryable:true on its own. Providers wrap any
unexpected error as retryable, so a TypeError in provider code, a 200 HTML page, a
null JSON body, a stream:false regression and a scheme-less base URL all skipped green.
An LLMError with no cause is still accepted, because the provider classified it from
an HTTP status or its own timeout. When there is a cause, the chain must contain a
retryable status, a socket/undici code, or an abort/timeout. New spec cases cover a
TypeError cause, a SyntaxError cause, and undici's fetch failed -> ECONNREFUSED.

The e2e now wraps the provider in withRetry with isRetryableLLMError, as the pipeline
does, so one 503/429/reset/timeout is absorbed and not reported as a regression. Errors
that outlive the retries are recorded. An untouched chunk skips only when every
recorded error is a proven outage. Otherwise it fails, and the message names the chunk
and the errors with their cause chains, or says no provider error was recorded. The
up-front probe and the re-probe are removed: the recorded errors answer the same
question, and the maxTokens: 1 probe could fail a thinking model red.

A 45s per-call timeout and a per-test breaker bound a mid-run hang to one retry budget
(about 140s), which fits under the 240s test timeout. Under CI, OLLAMA_BASE_URL must be
set. Every unreachable skip emits a ::warning:: annotation. Stale comments are
corrected, and the no-op callProvider wrapper around enhanceAndCategorize is removed.

Verified against a fake Ollama. Healthy and one-503 runs pass. HTML, null JSON, an
injected provider TypeError, a removed stream:false and scheme-less URLs fail red with
the cause chain. A dead port skips with ::warning::. A hang after the first chunk
skips after about 138s per test. CI=1 with no base URL fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): make the skip annotation parse under CI and trim unreachable predicate branches

Third-review follow-ups:

- Under CI vitest colours the `stdout | <test>` header, leaving escape
  codes in front of `::warning`, so the runner never parsed it as an
  annotation — every skip passed silently. Emit it on its own line, and
  pass the reason to ctx.skip().
- isProviderUnreachable: drop the status-in-cause and abort/timeout-name
  branches and the undici headers/body timeout codes — the Ollama
  provider classifies statuses and its own timeout itself, so they were
  unreachable; add EHOSTUNREACH/EHOSTDOWN, which production retries.
  Document the inherent blind spot (a provider-classified 5xx or timeout
  is trusted).
- Assert no provider error was recorded when both chunks were rewritten;
  de-duplicate recorded errors by description; correct the breaker
  comment; size the test timeout for a chunk that needed corrective
  calls before a hang.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): drop a pass-path assertion that can't fail; note the per-test skip decision

Fourth-review nits: any recorded provider error already ends its chunk in
the verbatim fallback, so asserting none on the pass path guarded
nothing. Document that the skip-vs-red call is made per test rather than
per chunk.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): drop low-value e2e guards

Remove the depth bound on the cause-chain walk (error causes aren't
cyclic in practice) and the de-duplication of recorded errors by
description, which only trimmed repeated lines from a failure message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): drop the e2e breaker and host-unreachable codes

Without the breaker a full hang skips both tests inside the 330s timeout
(measured 276s and 138s). EHOSTUNREACH/EHOSTDOWN aren't in production's
NETWORK_ERROR_CODES; if one occurs the test goes red, the safe direction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* style: add braces to the new single-statement bodies

main now enforces style/useBlockStatements (#679).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): tighten LLM e2e comments

Keep the non-obvious why; drop restated mechanics and examples the code already shows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(notes): cut LLM e2e comments to the non-obvious

Drop comments that restate test names, test data, assertions or
self-explanatory helpers; shorten the rest. The unclassified-error test now
uses Ollama's real message instead of a comment pointing at it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(notes): call the LLM e2e what it is: a live Ollama test

It calls the notes tasks directly with an OllamaProvider, which makes it an
integration test against a live service, not an e2e (test/e2e drives the
packed CLI). Rename it to ollama-live.spec.ts and RELEASEKIT_NOTES_E2E[_MODEL]
to RELEASEKIT_NOTES_LIVE[_MODEL].

Its outage predicate assumes the Ollama provider's error shapes (SDK
providers attach HTTP errors as a cause), and this spec is its only user, so
it moves into the spec as isOllamaUnreachable. Its tests stay outside the
opt-in suite so they run in every gate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
goosewobbler added a commit that referenced this pull request Oct 23, 2026
…crate (#674)

* fix(version): sync Cargo.lock on every Cargo.toml bump path

The #496 lock sync only ran from updateCargoFiles, so crates bumped
through version.groups (grouped and ungrouped) or the per-package
processor left their own entry in the committed Cargo.lock at the old
version. The sync now runs inside updateCargoVersion, the one function
every version-step Cargo.toml write goes through.

The sync also stops shelling out to `cargo update --workspace --offline`,
which exits 101 on a runner with an empty CARGO_HOME (no cached crates.io
index) and needs a Rust toolchain the release job may not have. Core's
new syncCargoLockVersion edits the lock text the way cargo does for a
path package's version change: the entry's version line plus any
"name old-version" dependency references. Registry and git entries are
never touched, and a lock where the edit would change how cargo tells
same-named packages apart is left alone with a warning. Publish's
prepare-stage sync uses the same helper.

Closes #650

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(version): sync every committed Cargo.lock that records a bumped crate

#650 now asks that no committed Cargo.lock needs a follow-up sync PR after
a release.

- Locks that use a crate through a path dependency are synced too: for
  each bumped crate, every Cargo.lock tracked by git with a sourceless
  entry of that name is updated, leaving registry and git entries of the
  name alone. The crate's own lock is still synced even when untracked.
  The tracked locks are listed once per run through the git seam (new
  Git.lsFiles).
- updateCargoVersion records each bump and VersionEngine.run syncs the
  locks once the strategy is done, so every strategy is covered, a lock
  shared by several crates is rewritten once, and every edit is worked
  out before the first write.
- A lock that can't be synced fails the run with CargoLockSyncError
  (CARGO_LOCK_SYNC_ERROR, VERSION_ERROR exit family) instead of warning.
  A dry run works out the same edits, so it fails too, and records them
  as pending writes that release's flush writes with the manifests.
- Publish's prepare stage syncs and stages every such lock, so the
  direct-commit flow commits fixture locks as well. It stays non-fatal:
  in the standing-PR flow it runs after the release PR has merged.
- The real-cargo integration test runs `releasekit version` with no
  cargo on PATH and an empty CARGO_HOME for version.groups, the
  per-package processor and updateCargoFiles, and checks that
  `cargo metadata --locked` accepts every such lock unchanged.

The syncCargoLockfile export of @releasekit/version (added for the
integration test in #498) is removed: the sync is now per run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(core): keep cargo's order when a lock bump passes another version

Cargo orders same-name [[package]] entries by semver and a package's
dependency references by the version as a plain string, so
"dupe 10.0.0" sorts before "dupe 2.0.0" (checked with cargo 1.97).
Rewriting a path crate's version in place left both in their old order
whenever the new version passed a same-named registry or git entry, and
the next cargo run would reorder the lock.

setCargoLockVersion now moves the edited entry, and the run of
references around each edited reference, back to cargo's order. The
real-cargo byte-for-byte test bumps crate-b from 0.1.0 to 0.2.0 past
git copies at 0.1.5 (entry order) and 0.10.0 (reference order).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* style: add braces to every single-statement body

Same sweep as #679, run on this branch so merging main resolves cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(version): sync a lock entry only for the crate it records, never an ignored lock

Crates in different workspaces can share a name (every `tauri init` app
is `app`), and every bump was applied to every lock with a path entry of
that name, so one app's lock took another app's version.

A crate's own lock still takes the crate's bump whatever version it
records. In any other lock, the entry follows a bump only when something
in that lock depends on it (an entry nothing depends on is that lock's
own root or member, a different crate) and it is at the crate's old or
new version; bumps record the manifest's pre-bump version for this.
Same-named bumps to different versions that fit one entry fail the run.

A crate's own lock that git ignores is no longer synced or staged: the
direct-commit release's `git add` refused it and failed the release.
`Git` gains `isIgnored` (`check-ignore -q`); the planner now does its
own discovery, so `findCommittedCargoLockfiles` is gone.

Also: version's syncCargoLockfiles returns void, the unused `git`
parameters and FakeGit's trackedFiles seed are dropped, the duplicate
comment in VersionEngine.run is removed, and tests pin numeric
pre-release ordering and the several-path-entries refusal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs: document CARGO_LOCK_SYNC_ERROR and how lock entries are matched

docs/cli.md lists the code (category `cargo-lock-sync`, exit code 8).
docs/rust.md states which lock entries follow a bump and covers every
reason the sync refuses a lock, not only the other-source one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs: keep the exit-code table to families; shorten the lock-sync bullet

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(version): treat a lock beside a crate's manifest as its own

findCargoLockfile doesn't read an ancestor workspace's `exclude`, so a
crate excluded from it (a Tauri app's src-tauri, say) got the ancestor's
lock as its own, and its real lock was matched as another workspace's:
its root entry, which nothing depends on, was skipped and left stale. A
lock beside the manifest can only be the crate's own (or an unused
stray), so it now counts as one.

When the manifest records no version (`version.workspace = true`, or
none), the old version now comes from the crate's own lock, so other
locks are still matched on it; only without an own lock does any
version match.

Tests pin both, the any-version fallback, publish reporting the
replaced version, and the versioned-reference match in core.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(rust): narrow the same-name claim, note the residual case and exit codes

Only other apps' own crates of the same name are told apart; a different
same-named crate that a lock depends on at the bumped crate's old or new
version is not, so the unsupported list says so and how to avoid it. The
lock-sync error exits 8 from `release` and `standing-pr` but 1 from
`releasekit version`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(rust): shorten the same-name limitation

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(core): find a crate's Cargo.lock by cargo's workspace-root rule

findCargoLockfile took the first ancestor `[workspace]` as the crate's
root without reading its `exclude`, so a crate excluded from the repo's
workspace treated that workspace's lock as its own, where every bump is
applied: two excluded Tauri apps named `app` failed the run as ambiguous
in a root lock that has no `app`, and bumping one excluded `app` set the
root workspace's own member `app` to its version.

It now follows cargo: a crate whose manifest has a `[workspace]` is its
own root; otherwise the nearest ancestor workspace that doesn't exclude
it is (an `exclude` path prefix not overridden by a `members` one, both
literal); with none, the crate is its own root. Only the root's lock is
returned, which also drops the nearest-lock fallback for standalone
crates (cargo never reads a non-root ancestor's lock). Checked against
cargo 1.97. `exclude` is read with smol-toml, now a core dependency.

The round-2 "lock beside the manifest" own-lock candidate is dropped: the
new rule returns it whenever it is the crate's lock, and a member's stray
lock is unused by cargo.

The planner's doc now notes that any version matches when neither the
manifest nor the crate's own lock records the old one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(rust): note the any-version case of the same-name limitation

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(core): pin whole-component exclude matching; note the unfollowed workspace pointer

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* refactor: cut Cargo.lock sync comments to the non-obvious

Drop doc comments that restate a name or signature, issue numbers and
history; shorten the rest to the cargo behaviour the code relies on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(core): sync another workspace's Cargo.lock only if it reaches the crate by path

Name and version can't identify a path crate: two workspaces can each have
a `core` 0.1.0, and bumping one rewrote the other's lock, breaking
`--locked` builds. A lock other than the crate's own now takes a bump only
if its workspace reaches the crate's directory through path dependencies
(target-specific, `workspace = true`, `[patch]`; dev-dependencies of its
own members only), as cargo resolves them. Drops the version heuristic and
`fromVersion`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(core): follow package.workspace pointers, and only the root's [patch]

Cargo honours [patch] only in the workspace root's manifest, so following
it from every visited manifest could rewrite an unrelated same-named entry.
And a member outside its workspace's directory points back with
package.workspace; without following it, its real lock went stale.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(core): refuse to edit a Cargo.lock format newer than v4

Also document that `[patch]` in `.cargo/config.toml` isn't followed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(core): stop following [patch] when syncing Cargo.lock

Cargo drops a patch the bumped version no longer satisfies, so following
it could write a lock cargo never would. Support is tracked in #706.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
goosewobbler added a commit that referenced this pull request Oct 23, 2026
…pipe (#616)

* feat(cli): envelope the version and publish commands, unwrap on the pipe

Completes the envelope scope of #544. `version --json` and `publish --json` were
the two commands still printing a bare payload; both now emit the uniform envelope
and accept `--output <path>` for parity with the orchestration commands. `notes`
has no JSON output, so the surface is now uniform.

The coupling this was deferred for is the pipe: `version | notes | publish` carries
JSON between processes, so enveloping the producer means teaching the consumers to
unwrap on input. Both do, via `unwrapEnvelope` in core — called once inside
`parseVersionOutput` and `parseInput` so the string, file, and stdin entries are all
covered. A bare `VersionOutput` still passes through, so a hand-assembled file or an
older releasekit keeps working.

Piping a failed stage forward now fails with that stage's message rather than a
schema complaint about missing fields, which pointed at the wrong thing.

Two things fell out of doing this honestly:

`errorEnvelope` takes optional `data` and `changed`. Enveloping publish's failure
path would otherwise have dropped `partialOutput` — what already landed before the
failure — leaving the enveloped output less informative than the prose it replaced,
and reporting `changed: false` for a partial publish that did change state. Publishes
are idempotent and retried, so that is exactly what the retry reads. `failedStage`
has no envelope field, so it is named in the error message rather than dropped.
Readers that ignore `data` when `status` is `error` are unaffected, so schemaVersion
is unchanged.

The publish-changed predicate moved to `@releasekit/publish` as `publishDidChange`.
The publish CLI and `standing-pr publish` must answer `changed` the same way, and
the logic is subtle enough (tags, `git.pushed`, and already-existing GitHub releases
all report success without anything having published) that two copies would drift.
It is also now tolerant of absent registry arrays: it runs on the partial output of a
failed pipeline, where throwing would replace the publish error the caller needs.

Note `version` now exits with the mapped code from the documented table rather than
always 1 — in practice only `GIT_ERROR`, which becomes 7.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* fix(cli): complete the exit-code table and validate envelopes on unwrap

Both from Greptile review of #616; both were real.

**Exit classification.** The code → exit-code map only held the nine family names, but
packages throw specific codes — NOT_GIT_REPO, TAG_ALREADY_EXISTS, PACKAGES_NOT_FOUND,
NPM_AUTH_ERROR and the rest — so almost every real failure fell through to exit 1 while
docs/cli.md advertised a mapping. Automation could not tell a git failure from a config
one. Every code the version, notes, and publish packages throw is now mapped to its
family, with a test that fails when a new code isn't, since the degradation is silent.

The family assignment is a judgement call in two places worth a second opinion:
workspace/package-discovery failures (PACKAGES_NOT_FOUND, WORKSPACE_ERROR) are treated
as config errors on the reasoning that the `packages` globs matched nothing, and
manifest-parse failures (CARGO_TOML_ERROR, PUBSPEC_YAML_ERROR) likewise.

**Envelope validation.** `unwrapEnvelope` accepted anything with the three keys, so an
envelope from a newer producer, an unrecognised status, or a success envelope with a null
payload all flowed through — the last surfacing in `notes` as a null-property error about
`changelogs` rather than an input error. It now rejects each with a message naming the
actual cause. This is what schemaVersion was for; nothing had been reading it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* fix(cli): report publish commits as changes and exit publish by code family

Review follow-ups for the envelope work:

- `publish --json` reported `changed: false` after creating the release
  commit and tags whenever no registry published (GitHub-release-only,
  `--skip-publish`). `publishDidChange` now also counts `git.committed`,
  which is only set for a commit this run created; `standing-pr publish`
  never commits (`skipGitCommit`), so its `changed` is unaffected.
- `publish` exited 9/1 regardless of the error code, contradicting the
  code-to-exit table. It now exits via `exitCodeForError`, like
  `version`: config failures exit 2, input failures 3, pipeline
  failures still 9.
- `version --output` with a relative path landed inside `--project-dir`
  (the command chdirs first). Resolve it against the invocation
  directory up front, as `release --output` does.
- docs: the `version | notes | publish` pipe never worked (`notes` does
  not pass the VersionOutput on); document chaining through a file.
  Correct the `changed`/retry wording and the exit-code example. Update
  the version and publish READMEs for the envelope and `--output`.
- release: reuse core's `writeEnvelope` instead of a private copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(cli): correct the chaining, retry and exit-code docs; move the --output test into CI

Second-review follow-ups:

- docs: the file-based `version` -> `notes` -> `publish` chain left the
  changelog out of the release commit (publish commits only the version
  files plus what is staged). Document staging it; verified end to end.
- docs: a re-run of the standalone `publish` in the same tree stops at
  the git-commit stage, so drop the blanket "a retry is safe".
- docs: the exit-code examples named codes `version` never throws
  (TAG_ALREADY_EXISTS / GIT_PROCESS_ERROR are unreachable). Use codes
  `publish` actually emits: input -> 3, config -> 2, pipeline -> 9.
- test: the `--output` integration test lived in version's integration
  suite, which CI never runs. Replace it with a unit test in
  test/unit/cli.spec.ts (fails against the pre-fix command).
- publishDidChange docstring: `committed` is also set in a dry run; the
  dry-run guard is what keeps that out of `changed`.
- version README: the flag table listed --strict-reachable, --verbose and
  --quiet, which the command doesn't have, and omitted several it does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(cli): scope the exit-code contract to the commands it holds for

Third-review follow-ups:

- The specific-code / family-exit contract holds for version, notes and
  publish, but `release` and `standing-pr` report errors raised inside
  their bundled version/notes/publish stages as GENERAL_ERROR (exit 1):
  the release bundle carries several copies of core's ReleaseKitError,
  so the instanceof check misses errors from the other copies. Say so
  rather than claim otherwise; the cause predates this PR and is fixed
  separately.
- `data` on error: describe the actual exception (a publish pipeline
  failure carries partial output even when nothing landed).
- core README: VersionOutput is now the envelope's `data`.
- Restore unit tests for releaseDidPublish, the standing-pr `changed`
  wiring, dropped when the predicate moved to @releasekit/publish.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(cli): don't crash the error path on an unwritable --output; scope the notes exit claim

Fourth-review follow-ups:

- docs/cli.md claimed the family exit-code contract for `notes`, but
  notes exits 1 for config errors and most non-parse failures; only its
  input-parse failures exit 3. Say exactly that.
- An unwritable `--output` path made the result write throw, then the
  error path threw again writing its envelope to the same file, crashing
  with a stack and no envelope. writeEnvelope still throws for a result
  write (the command must fail), but notes a failed error-envelope write
  on stderr instead of throwing out of the error handler.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(cli): describe notes' parse check precisely; name a lost --output result

Fifth-review nits:

- notes only checks that its input has a `changelogs` array, so "a
  schema-invalid object" overstated the exit-3 case; say what it checks.
- When the result write to --output fails, throw "Could not write the
  result to <path>: ..." instead of the bare ENOENT, so stderr says the
  command's work happened but its result never reached the file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test: drop low-value review additions

Remove the releaseDidPublish unit tests (a one-line wrapper over
publishDidChange, which publish's own tests cover) and rethrow the
original error when a result can't be written to --output instead of
re-wrapping it — the ENOENT already names the path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test: drop the duplicate publish exit test and the notes exit clause

INPUT→3 runs the same exit-code path as CONFIG→2, which stays as the guard
against hard-coded exits. The cli.md notes clause is trimmed to the commands
the table holds for.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(cli): release and standing-pr now report a stage's own error code

#651 is merged, so an error raised inside their version, notes or publish
stage keeps its code instead of degrading to GENERAL_ERROR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* refactor(cli): share one error-envelope writer and trim review leftovers

- core: add writeErrorEnvelope next to writeEnvelope; version, publish and
  release's emitError now call it instead of each building the same
  single-error envelope.
- core: isEnvelope is no longer exported — only unwrapEnvelope needs it, and
  the version test helper now reads output through unwrapEnvelope, the same
  boundary notes and publish cross.
- publishDidChange: drop the `?.` / `?? []` guards. Every PublishOutput it
  sees comes from runPipeline's ctx.output (the only PipelineError is built
  from it), whose registry arrays are always initialised.
- Drop comments that repeat the unwrapEnvelope JSDoc or another comment,
  the old-behaviour note on publish's exit, and the "pipe's contract is
  unchanged" claim (older consumers reject the envelope).
- Braces on the one-line ifs and loop this PR added; give publishDidChange
  its own export section so runPipeline keeps its header.
- Drop the "older producer" unwrap test: it used the current
  schemaVersion, so it duplicated the success-envelope case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(cli): cover the version and publish failure paths through the commands

- version --json: an engine error reaches stdout as an error envelope with
  its own code and exits by its family (INVALID_CONFIG -> 2); --stable with
  --prerelease prints an INPUT_ERROR envelope and exits 3.
- publish --output: a PipelineError writes its envelope, partial output in
  `data`, to the file and nothing to stdout.
- publish --input holding a failed upstream envelope runs the real
  parseInput and exits 3 with INPUT_PARSE_ERROR. It replaces the "registry
  failure" test, which rejected runPipeline with a bare NPM_PUBLISH_ERROR
  that runPipeline never throws (it wraps every stage error in a
  PipelineError).
- Exit-code coverage moves from a hardcoded list in core to each package's
  own error enums (PublishErrorCode; VersionErrorCode, GitErrorCode and
  StrictReachableError), so a new unmapped code now fails a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs: point --json stability at schemaVersion; keep the version README to this PR's flags

- troubleshooting: "the JSON schema is stable across minor versions" no
  longer holds — this release changes the version/publish --json shape. Say
  what is versioned instead: the envelope's schemaVersion.
- version README: revert the flag-table rewrite that rode along (short
  aliases, --stable, --allow-first-bump, --sync, --config,
  --include-prerequisites, and dropping --strict-reachable / --verbose /
  --quiet). Keep only the --json description and the new --output row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* style: add braces to every single-statement body

Same sweep as #679, run on this branch so merging main resolves cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* fix(cli): suppress per line instead of exempting cli.ts; test --output alone

The --json print takes a one-line noConsole suppression and the unwritable
error-envelope note goes through the logger, so core/src/cli.ts no longer
needs a file-wide exemption. A version --output test now fails if --output
alone stops collecting the result. Inline releaseDidPublish, drop a guard
writeEnvelope already applies, and trim two comments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(version): write --output through the real JSON writer

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs(core): trim the envelope doc comments; drop a stale e2e helper note

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* docs: tighten the envelope docs and comments

Fix the failed-upstream wording in docs/cli.md (it's an input error that names
the upstream code), say plainly that a partial publish can't be resumed yet,
and cut or shorten the comments that restated code or docs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: link to the chaining section from architecture, simplify envelope comments

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
goosewobbler added a commit that referenced this pull request Oct 23, 2026
* test(notes): add the structured enhance-and-categorize eval case

Completes the assertion scope of #542. The harness shipped with the free-text
release-notes path; the structured path is the case it was built to take next, and
the one that unlocks the category-distribution assertion the issue lists.

Two assertions the free-text case can't reach:

`checkCategoryDistribution` — grouping has to discriminate, not merely be legal. The
task validator already rejects category names outside the configured set, so a model
that drops every entry into one bucket passes validation and still produces a useless
grouping. Catches too few populated categories, an emitted-but-empty category, and one
category swallowing most of the entries. The share bound is skipped below four entries,
where a lopsided split isn't evidence of anything.

`checkNoEntryLoss` — every entry survives categorization exactly once, catching both
drops and duplication across buckets.

`loadGoldenCase` is now generic in its context type, since the tasks take different
ones — release notes a ReleaseNotesContext, this an EnhanceContext & CategorizeContext.

Also adds unit tests for the assertion module. These are the regression net, so each
needs a case that actually trips it; without one the eval would pass every run while
catching nothing.

Verified the drift signal fires on the new case: editing the enhance-and-categorize
system prompt changes the cache key and the run fails with the re-record hint rather
than silently replaying stale output. Recording the new case left the existing
release-notes fixture untouched, confirming the per-case refresh.

Part of #542.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* test(notes): give the eval categories their required descriptions

From Greptile review of #617, and a real defect in the fixture.

`LLMCategory.description` is required, and `buildCategorySection` renders each
category as `- "<name>": <description>`. The fixture omitted them, so the prompt the
eval measured against contained `- "New": undefined` — a shape production never
produces. The case was pinning malformed categorization guidance.

Re-recorded from a clean cache, since the corrected prompt changes the key. That also
drops the entry orphaned under the old key — the clean-slate procedure the harness
header documents, now exercised for the first time. The release-notes fixture is
untouched, confirming per-case recording.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27

* test(notes): check entry identity in the eval and use a production-shaped golden input

Review follow-ups for the structured eval case:

- `checkNoEntryLoss` compared totals only, so a dropped entry and a
  duplicated one cancelled out (a groupByCategory mutation that did
  exactly that passed the eval). It now checks identity: every enhanced
  entry sits in exactly one bucket once, no bucket holds anything else,
  and enhancement kept the golden entry count. The same mutation now
  fails with "entry 0 was categorized 2 times / entry 1 was dropped".
- The golden input kept raw `feat(api): ...` subjects with no `scope`,
  a shape neither commit parser produces, so the pinned prompt wasn't
  the one production builds. Use the parsed shape (bare subject,
  `scope`, `originalType`, no empty `issueIds`) and re-key the cache from
  the hand-authored recorded response.
- A cache miss on a corrective retry now says the recorded response was
  rejected, instead of advising a re-record that would fail identically.
- Note that the case exercises the text -> JSON path, not the
  structured-output branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): tighten the eval harness and fixture after second review

- Record mode answered corrective retries with the canned markdown,
  caching orphan entries that replay then served, so a rejected
  recording failed later on a misleading distribution assertion. Both
  offline modes now refuse a corrective retry at the source.
- checkNoEntryLoss reports grouping that copies entries as a broken
  identity assumption instead of N phantom "dropped" entries.
- Restrict the golden case's scopes to api/config, so grouping has to
  use the scope-validated entries: grouping the pre-validation entries
  now fails the eval (it passed before). Prompt and cache key unchanged.
- Retarget the prefix-leak guard to a leading `word(scope):` / `word:`
  prefix; the old pattern only matched tokens the prompt never contains.
- checkCategoryDistribution: drop the possibly-undefined reduce seed
  (TS18048 under tsconfig.test.json).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): only call bucketed entries copies when their content matches

Third-review follow-ups:

- checkNoEntryLoss's "copies" diagnosis matched on counts alone, so
  bucketing the raw, un-enhanced input (a real regression) was reported
  as a harmless copy and the developer told to relax the check. It now
  requires each stray to be bucketed once and to match a distinct
  missing entry by content; anything else reads as loss. Partial copies
  are named as copies; unrelated objects and duplicated copies are not.
- Assert directly that no disallowed scope reaches a bucket, so grouping
  the unvalidated response fails on its own, not only via identity.
- Build the prefix-leak pattern from the golden case's own type,
  originalType and scope labels, so a legitimate lead-in ("CLI: ...")
  in a live run isn't flagged.
- checkCategoryDistribution's doc listed two failure modes; it checks
  three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): close the last copy-detection and scope-check gaps

Fourth-review nits:

- Cover the length guard in isOneToOneCopy: one copied entry plus one
  genuinely dropped entry must read as loss, not as a clean copy that
  hides the drop.
- Compare copies by key-sorted JSON, so a copy rebuilt with its keys in
  another order is still recognised.
- Match scopes the way production validates them (case-insensitive
  unless the config says otherwise), so a live model's "API" for an
  allowed "api" is not a false failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): drop low-value eval additions

Remove the copy diagnosis in checkNoEntryLoss (isOneToOneCopy,
canonicalJson and their spec cases) — the check fails either way, it
only changed the message for a refactor that doesn't exist — the regex
escaping of the fixture's fixed labels, and the case-insensitive scope
match, which only mattered for live-model runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* style: add braces to the new single-statement bodies

main now enforces style/useBlockStatements (#679).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e

* test(notes): tighten eval comments

Keep the non-obvious why; drop restated mechanics and examples the code already shows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(notes): trim eval comments and escape the prefix-leak labels

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

Enforce block statements with Biome's useBlockStatements

2 participants