Skip to content

feat(lint): surface two-fix heuristic on pinned warning-count failures (gh-ludics-497)#501

Merged
lukstafi merged 1 commit intomainfrom
ludics/gh-ludics-497-s1/root
May 5, 2026
Merged

feat(lint): surface two-fix heuristic on pinned warning-count failures (gh-ludics-497)#501
lukstafi merged 1 commit intomainfrom
ludics/gh-ludics-497-s1/root

Conversation

@lukstafi
Copy link
Copy Markdown
Owner

@lukstafi lukstafi commented May 5, 2026

Summary

  • Adds formatWarningCountHeuristic(n) in scripts/lint-test-isolation.ts and wires it into both the CLI summary (when warningCount > 0) and the pinned-count integration test as a guarded throw before expect(...).toBe(20). Bun's failure surface now prints the actionable heuristic ("wrap with withSyntheticHarness(beforeEach, afterEach) OR bump the pin") ahead of the numeric matcher diff.
  • Mirrors the same idiom on the sibling lint-contracts pin via formatContractsHeuristic(n), with distinct wording (no withSyntheticHarness mention; AC4 invariant).
  • Captures two filter-rejected doctrine items (named-lint enumeration; optional pre-commit hook) as docs/swe-textbook.md entries per Capture Idempotency.
  • Zero-warning CLI summary preserved byte-identical to today (AC2 negative invariant).

Closes #497.

Test plan

  • bun run typecheck clean
  • bun test — 2245 pass / 0 fail (was 2243 at baseline; +2 from AC2 positive/negative tests)
  • bun run lint:test-isolation summary on live tree:
    ✅ No test-isolation anti-patterns detected (20 warnings — to silence a new test: wrap with withSyntheticHarness(beforeEach, afterEach) from src/test-utils.ts (preferred — actually isolates the harness env), OR bump the pinned count in scripts/lint-test-isolation.test.ts (only for pure-unit tests with no harness needs).
  • bun run lint:contracts summary unchanged byte-for-byte
  • bun run lint clean
  • AC1 verifier: git grep -F "wrap with withSyntheticHarness(beforeEach, afterEach) from src/test-utils.ts" finds the literal in exactly one production source (scripts/lint-test-isolation.ts formatter body) plus one test-side toContain literal, no copy-paste in the integration-throw site
  • AC4 verifier: git grep -F "withSyntheticHarness" -- scripts/lint-contracts.ts scripts/lint-contracts.test.ts returns zero hits
  • AC5 verifier: both new headlines (### Cherry-picking one named lint… and ### "Optional pre-commit hook"…) present in docs/swe-textbook.md with required Description: / Precipitating retro: / Filter decision: fields
  • AC7/AC8/AC9/AC10 invariants: branch diff covers exactly five files (scripts/lint-{test-isolation,contracts}{,.test}.ts + docs/swe-textbook.md); no edits under src/coder/**, src/reviewer/**, src/orchestration/**, agent-loaded skills, .husky/**, or package.json's husky/lint-staged keys

Post-merge

Per AC6, comment on issue #497 with the merged-PR link plus the two docs/swe-textbook.md anchor IDs (verify literal slugger output via gh api -H "Accept: application/vnd.github.html" repos/lukstafi/ludics/contents/docs/swe-textbook.md?ref=<merge-sha> before pinning slugs), then close.

🤖 Generated with Claude Code

…s (gh-ludics-497)

Adds an exported `formatWarningCountHeuristic(n)` to scripts/lint-test-isolation.ts
and wires it into both the CLI summary line (when warningCount > 0) and the
pinned-count integration test as a guarded throw before `expect(...).toBe(20)`.
Now Bun's failure surface prints the actionable heuristic — "wrap with
withSyntheticHarness(beforeEach, afterEach) OR bump the pin" — ahead of the
numeric matcher diff. The zero-warning summary is byte-identical to today.

Mirrors the same idiom on the sibling lint-contracts pinned warning count via
`formatContractsHeuristic(n)`, with contracts-specific wording (no
withSyntheticHarness mention) seeded from the existing integration-test
comment.

Captures two filter-rejected doctrine items as docs/swe-textbook.md entries
(per Capture Idempotency procedure): "Cherry-picking one named lint" (filtered
action 1) and ""Optional pre-commit hook" feedback-digest items must verify
the infra exists" (filtered action 3).

Regression coverage: two new AC2 unit tests in
scripts/lint-test-isolation.test.ts (positive: rule-3 fixture asserts the
summary contains the heuristic; negative: clean fixture asserts byte-identity
with the pre-fix summary). Integration-test guarded throws use
formatWarningCountHeuristic / formatContractsHeuristic, so the heuristic body
lives in exactly one production source per lint family.

Closes #497.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lukstafi
Copy link
Copy Markdown
Owner Author

lukstafi commented May 5, 2026

@codex review Focus on bugs, correctness issues, and edge cases. Do not check adherence to a spec or plan.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@lukstafi lukstafi merged commit f53a85c into main May 5, 2026
1 check passed
@lukstafi lukstafi deleted the ludics/gh-ludics-497-s1/root branch May 5, 2026 18:57
@lukstafi
Copy link
Copy Markdown
Owner Author

lukstafi commented May 5, 2026

Coder retro — what I'd do differently next time

Three small process refinements surfaced by this PR. None block follow-up; logging here for future rounds.

1. Run the AC's literal verifiers against the current tree before writing "no gaps"

The reviewer's REQUEST_CHANGES on merge-0 caught one real failure mode: I declared "No assumption gaps" without first running AC5's literal grep -F "<PRECIPITATING_RETRO>" against the current docs/swe-textbook.md, which would have flagged the task-a670cdbf collision with Entries A/B from gh-ludics-496. The fix was a 5-line edit, but the cheaper path is to make every AC verifier whose passing condition depends on "the world" (textbook content, real-tree warningCount, live skill files) a pre-write probe rather than a post-write check. Cost: a couple of minutes of grep up front. Benefit: the plan ships with the gap pre-resolved instead of round-tripping through reviewer feedback.

2. AC1's "constant or formatter" wording invites unnecessary aliasing

My coder plan initially proposed both formatWarningCountHeuristic AND a WARNING_COUNT_HEURISTIC = formatWarningCountHeuristic alias, reading AC1's "single named constant ... WARNING_COUNT_HEURISTIC" suggestion as load-bearing. The merged plan correctly cut the alias — AC1 explicitly accepts "the constant (or a small formatter wrapping it)", so the formatter alone satisfies both the literal text and the verifier. Lesson: when an AC's suggested-name is a suggestion, don't add a second name to satisfy both readings; pick the simpler shape and cite the parenthetical.

3. The contracts-side CLI-summary asymmetry is a small follow-up worth declaring up-front

I left scripts/lint-contracts.ts's runCli summary un-wired (only the integration-test guarded throw landed), per AC4's literal scope. The asymmetry is defensible — the contracts CLI is rarely consulted directly, while lint:test-isolation is — but if a future round wants symmetric ergonomics it's a ~5-line edit (replicate the Step 2 branching pattern with formatContractsHeuristic in place of formatWarningCountHeuristic). I should have declared it explicitly as a scope-expansion: lift CLI summary wiring into lint-contracts for symmetry candidate so the reviewer could absorb it in this PR if preferred. Net: tiny PR-surface change, but cleaner to surface the option than to bury it as an implementation choice.

What went well (carry forward)

  • The exported-formatter-with-template-substitution shape (formatWarningCountHeuristic(n)) made AC1's "single source of truth" verifier mechanically falsifiable — git grep -F returns hits only at the formatter body and at test sites that call the formatter, never a copy-paste literal.
  • The byte-identity assertion on the zero-warning summary path (expect(summary).toBe(...) rather than toContain) is the minimal guard against accidental whitespace drift; cheap and falsifiable.
  • Single-message commit + heredoc PR body kept the diff small (5 files, +109 / -15 lines) without losing the AC self-check trail.

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.

lint:test-isolation pinned-warning count breaks CI on every new test file

1 participant