Skip to content

bench: fix unbalanced parens in 5 micro-bench files - #29

Merged
hierophantos merged 1 commit into
mainfrom
fix/bench-paren-errors
Apr 27, 2026
Merged

bench: fix unbalanced parens in 5 micro-bench files#29
hierophantos merged 1 commit into
mainfrom
fix/bench-paren-errors

Conversation

@kumavis

@kumavis kumavis commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Each affected line had one extra trailing ) from a paste-error pattern introduced in d7bd97a ("PPN 4C S2.e-iv-c: retire 6 store/champ-box parameters + 169-test-file fixture surgery").

raco pkg install --auto --skip-installed runs raco setup, which compiles the whole package — including benchmarks/micro/. With these files broken, install exits non-zero, breaking CI on main and on every open PR (including #28, where the failure first surfaced).

Changes

22 lines changed (one extra ) removed on each):

  • bench-ppn-track2.rkt:412, 441
  • bench-ppn-track3.rkt:86, 294, 594, 630, 666, 685, 694
  • bench-ppn-track4.rkt:321, 325, 329, 333, 337, 341
  • bench-sre-track2d.rkt:317, 321, 325, 329
  • bench-sre-track2h.rkt:329, 341, 357, 370

No semantic change.

Test plan

  • Each touched file: raco make benchmarks/micro/<file>.rkt → exit 0
  • Full bench dir: for f in benchmarks/micro/*.rkt; do raco make "$f"; done → all clean
  • CI: Tests workflow's Install dependencies step succeeds (this is the step the bug was blocking)
  • CI: full test suite still passes (no semantic change, expected)

https://claude.ai/code/session_01YM6gc3cMNH2Ymor4jdZY8u

Generated by Claude Code

@claude
bench: fix unbalanced parens in 5 micro-bench files
ab90c44 
Each affected line had one extra trailing `)` from a paste-error
pattern introduced in d7bd97a ("PPN 4C S2.e-iv-c: retire 6 store/
champ-box parameters + 169-test-file fixture surgery").

raco pkg install --auto --skip-installed runs raco setup which
compiles the WHOLE package, including benchmarks/micro/. With these
files broken, install exits non-zero on every PR, blocking CI.

22 lines changed (one extra `)` removed on each):

  bench-ppn-track2.rkt:412     (process-file f)))))
  bench-ppn-track2.rkt:441     (process-file f)))))
  bench-ppn-track3.rkt:86      (process-string-ws src)))))
  bench-ppn-track3.rkt:294     (process-string sexp-simple)))))
  bench-ppn-track3.rkt:594     (process-file f)))))
  bench-ppn-track3.rkt:630     (process-file f)))))
  bench-ppn-track3.rkt:666     (set! e3-pass (+ e3-pass 1))))
  bench-ppn-track3.rkt:685     (process-string sexp-simple)))))
  bench-ppn-track3.rkt:694     (process-string sexp-medium)))))
  bench-ppn-track4.rkt:321,325,329,333,337,341
  bench-sre-track2d.rkt:317,321,325,329
  bench-sre-track2h.rkt:329,341,357,370

Verification: every file in benchmarks/micro/ now `raco make`s
clean. No semantic change.

https://claude.ai/code/session_01YM6gc3cMNH2Ymor4jdZY8u
hierophantos
hierophantos approved these changes Apr 27, 2026
View reviewed changes

@hierophantos hierophantos left a comment

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.

Approving and merging — this is our bug, you're fixing it gratis. Apologies for the regression.

Root cause confirmed: commit d7bd97a4 (S2.e-iv-c, 169-test-file mechanical sed surgery) introduced extra ) on 22 lines across 5 bench files. Our 2-pass sed methodology covered tests + driver + lsp + tools/batch-worker but missed the bench/ tree, and our own discipline (.claude/rules/testing.md requires check-parens.sh after every .rkt edit) wasn't applied to files outside the targeted-tests scope.

The downstream impact you note — CI failure on every open PR — is on us. Thanks for surfacing it.

Acting on your suggestion: adding a pre-commit hook that runs tools/check-parens.sh on staged .rkt files (we have the tool + git-hooks infrastructure already; just need to drop the script in). That closes this specific bug class going forward.

PR-workflow-with-CI is a separate question we're considering but holding off on for now — the hook addresses the recurrence path; PR-workflow would catch a broader class but at heavier cost. Will revisit if more bug-class instances surface.

@hierophantos
hierophantos merged commit 014507b into main Apr 27, 2026
1 check passed
@hierophantos
hierophantos deleted the fix/bench-paren-errors branch April 27, 2026 20:22
hierophantos added a commit that referenced this pull request Apr 27, 2026
@hierophantos
hooks: add pre-commit paren-balance check on staged .rkt files
e58616e 
Closes the bug class that introduced commit d7bd97a's regression
(unbalanced parens in 5 micro-bench files broke `raco pkg install`,
breaking CI on every open PR; surfaced + fixed by kumavis in #29).

The hook runs tools/check-parens.sh on staged .rkt files via the
existing read-syntax-based delimiter check. ~100ms per file; blocks
commit on mismatch with exact line:column. Bypass with --no-verify
for emergencies (per workflow.md hook discipline).

Symlink-installed via tools/install-git-hooks.sh — edits to
tools/git-hooks/pre-commit propagate immediately. Existing post-commit
mempalace hook unaffected; both now installed by the same one-shot
installer.

Per-developer activation: ./tools/install-git-hooks.sh after cloning.
Existing developers: re-run the installer to pick up the new hook.

Documentation in .claude/rules/testing.md alongside the existing
pre-push gate entry.
hierophantos added a commit that referenced this pull request Apr 27, 2026
@hierophantos
dailies: document PR #29 fix + pre-commit hook landing
07609d0 
PR #29 from kumavis (014507b) — fix unbalanced parens in 5 micro-bench
files introduced by our commit d7bd97a (S2.e-iv-c sed surgery).
Acknowledgment of the regression + lesson distilled.

Pre-commit hook (e58616e) — closes the bug class for future
mechanical edits. Documentation already in testing.md § Pre-commit gate.
kumavis pushed a commit that referenced this pull request May 4, 2026
@claude
ocapn: Phase 17 — Error type for broken promises (3 tests)
3c8d2ed 
OCapN's wire convention for "promise broken with reason r" is
<op:deliver <desc:answer pid> <Error r> false false>. Phase 12
emitted bytes only for fulfilled promises (broken returned `none`).
Phase 17 closes the gap.

Module additions to captp-bridge.prologos:
  wrap-error : SyrupValue -> SyrupValue   ;; (syrup-tagged "Error" r)
  outbound-from-resolution now handles pst-broken via wrap-error.
  pump-one delegates to outbound-from-resolution (was only checking
  pst-fulfilled inline).

Phase 16's wire-OUT pump now correctly emits bytes for both
fulfilled AND broken promises in the same pass.

Test additions (3 cases, total 22 in test-ocapn-bridge.rkt):
  - outbound-from-resolution broken -> some
  - broken bytes equal `<op:deliver <desc:answer N> <Error r> false false>`
  - wrap-error byte-equals (syrup-tagged "Error" r)

Pitfall #29 added: `break` from prologos::ocapn::promise collides
with `break-helper` from prologos::data::list in sexp-mode
resolution. Workaround: use the constructor `pst-broken` directly
in tests instead of the convenience wrapper.

Tests: 22/22 green on Racket 9.1.

https://claude.ai/code/session_01YM6gc3cMNH2Ymor4jdZY8u
@kumavis kumavis mentioned this pull request May 4, 2026
Draft
kumavis pushed a commit that referenced this pull request May 4, 2026
@claude
ocapn: Phase 17 — Error type for broken promises (3 tests)
4e0c9c2 
OCapN's wire convention for "promise broken with reason r" is
<op:deliver <desc:answer pid> <Error r> false false>. Phase 12
emitted bytes only for fulfilled promises (broken returned `none`).
Phase 17 closes the gap.

Module additions to captp-bridge.prologos:
  wrap-error : SyrupValue -> SyrupValue   ;; (syrup-tagged "Error" r)
  outbound-from-resolution now handles pst-broken via wrap-error.
  pump-one delegates to outbound-from-resolution (was only checking
  pst-fulfilled inline).

Phase 16's wire-OUT pump now correctly emits bytes for both
fulfilled AND broken promises in the same pass.

Test additions (3 cases, total 22 in test-ocapn-bridge.rkt):
  - outbound-from-resolution broken -> some
  - broken bytes equal `<op:deliver <desc:answer N> <Error r> false false>`
  - wrap-error byte-equals (syrup-tagged "Error" r)

Pitfall #29 added: `break` from prologos::ocapn::promise collides
with `break-helper` from prologos::data::list in sexp-mode
resolution. Workaround: use the constructor `pst-broken` directly
in tests instead of the convenience wrapper.

Tests: 22/22 green on Racket 9.1.

https://claude.ai/code/session_01YM6gc3cMNH2Ymor4jdZY8u
kumavis pushed a commit that referenced this pull request Aug 3, 2026
@claude
feat(string): grapheme clusters — String Library Phase 4a
e61437e 
`str::grapheme-count`, `str::grapheme-span`, `graphemes`,
`grapheme-reverse`.

The entry's "Mitigation" line was the whole answer. It reads as though
UAX #29 tables were the work and an FFI bridge a fallback; the bridge IS
the implementation, because Racket already carries the tables. A ZWJ
family emoji comes back as one cluster from five code points. Two
`foreign racket` lines plus a walk over `grapheme-span`.

I had written this off in the same breath as sorted collections and
regex, on the strength of "~30KB Unicode tables" — while the line
directly beneath it said what to do instead. Third time this session that
"blocked" meant "stopped reading".

Tested where clusters and code points DIFFER, because ASCII proves
nothing: `graphemes "abc"` is right whether or not the implementation
understands clusters. The load-bearing case is reversing NFD "éa" by
grapheme and re-composing, which gives "aé" — a code-point reverse moves
the accent onto the "a" and composes to "áe" instead. Same length, same
grapheme count, different string; only comparing content catches it. One
assertion checks the fixture is actually decomposed first, so the test
cannot pass vacuously.

Suite 10224/531 green, conformance 24/24.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YM6gc3cMNH2Ymor4jdZY8u
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hierophantos hierophantos hierophantos approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kumavis @hierophantos @claude