fix: extinguish ignored review bugs, purge em dashes, add mermaid, v0.1.0 perf baseline#150
Merged
Merged
Conversation
…r, perfeval, flamegraph) Six real findings CodeRabbit flagged on merged PRs and we had not actioned: - qsl-bench profile_seconds_from_args (#135): !(seconds > 0.0) does not reject inf/nan from strtod, and converting a non-finite/unbounded double to clock_type::duration is UB. Now requires std::isfinite and clamps to 3600s. - tcp_server transient_accept_errno (#140): omitted ENOPROTOOPT and EOPNOTSUPP, which Linux accept(2) returns as already-pending per-connection errors and the epoll path already retries. They were wrongly fatal in the threaded acceptor; the set now matches is_transient_accept_error(). - qsl-perfeval ring (#148): submit() ringed every id, but marketable orders that fully fill never rest, so retire cancelled non-existent ids and book depth drifted with match rate. track() now checks eng.contains and rings only resting orders, holding the book genuinely ~kRing deep. - qsl-perfeval p99 (#148): (n*99)/100 selected the max for small n; now uses the zero-based ((n-1)*99)/100 (and p50 likewise). - qsl-perfeval parse_orders (#148): std::strtoull accepted "123abc"/"-1" (wraps to a huge count feeding reserve()/the loop). Now std::from_chars validates the whole token and rejects malformed input with a usage error + exit 2 (test added). - flamegraph.py [unknown] folding (#135): rewritten with a precise, identified rationale. The [unknown] frames are fp-unwinding artifacts (glibc 2.43 malloc fast paths do not preserve x29 -> a spurious frame between resolved operator-new and _mid_memalign/_int_malloc/cfree; plus the vDSO clock_gettime leaf). Each is folded into its resolved caller so every rendered frame is a real symbol and the true operator-new -> malloc chain is revealed. DWARF was verified worse (mangles the _start asm entry into ~3 unknowns/stack). make check/asan 272/272; CodeScene clean. Perf evidence (PERFORMANCE.md, flamegraphs) regenerated with the fixed harness in a follow-up commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…megraphs Re-measured PERFORMANCE.md after the qsl-perfeval ring fix (track only resting orders) on the same host, back-to-back before/after: throughput 9.25M -> 10.76M orders/sec (+16.3%) cycles/order 345.7 -> 297.3 (-14.0%) instr/order 1246 -> 1144 (-8.2%); IPC 3.60 -> 3.85 branch-miss 1.86% -> 1.69% allocs/order 1.108 -> 1.108 (unchanged) p50/p99 ~83 / ~208 ns both (latency distribution unchanged) cache-miss unavailable (Apple Silicon PMU; #90) Honesty corrections from the counters: the win is throughput + cycles/order, not the new_limit latency tail (the earlier 250->208 p99 was thermal warm-up; steady state is ~208 both). Allocations are unchanged (libstdc++ map::emplace checks before allocating). New section identifies every [unknown] frame (fp glibc-malloc boundary artifact + vDSO clock_gettime leaf) and documents that DWARF is worse; all flamegraphs now render with ZERO [unknown] and the libc malloc internals resolved. results/flamegraph.svg regenerated (0 unknown, Dirty inputs: no). PERFORMANCE.md and perf-stat.txt are em-dash-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…numbers Removed every em dash (U+2014) and en dash (U+2013) from all 66 tracked text files (CLAUDE.md, AGENTS.md, MILESTONES.md, PROGRESS.md, HANDOFF.md, every doc, CHANGELOG, source-comment prose, scripts, tests), replacing with readable ASCII punctuation (comma / colon / period / hyphen by context). 0 em/en dashes remain repo-wide (ocaml/test/fixtures verified clean already). Source changes are comment-only; build + check stays 272/272. Refreshed the README "numbers" block, which still carried the pre-remeasurement figures, to match the corrected PERFORMANCE.md (fixed qsl-perfeval harness): throughput 9.25M -> 10.76M (+16%), cycles/order 345.7 -> 297.3 (-14%), IPC 3.60 -> 3.85, branch-miss 1.86% -> 1.69%, allocs unchanged. Dropped the misleading p99 row (the latency distribution is unchanged; the win is throughput and cycles/order). Tests badge and quality table 271 -> 272. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The performance evidence now compares the FIRST RELEASE (v0.1.0) to current, the cumulative engine change, instead of just the two most recent micro-opts. The v0.1.0 column was produced by porting the same qsl-perfeval harness into a git worktree at the v0.1.0 tag (identical MatchingEngine API) and running the same release preset back-to-back on the same host. Cumulative (v0.1.0 -> v0.2.2, baseline storage): allocations/order 4.094 -> 1.108 (-73.0%) <- dominant cumulative win branch-miss rate 2.05% -> 1.69% (-17.5% relative) throughput 10.54M -> 10.99M (+4.3%) cycles/order 304.5 -> 290.7 (-4.5%); IPC 3.84 -> 3.94 p50/p99 latency ~83/~209 -> ~83/~208 ns (unchanged) cache-miss unavailable (Apple Silicon PMU; #90) Honest mechanism: the big change since v0.1.0 is allocation traffic (the storage/PMR work), cut 73% even on the default baseline path; throughput/cycles move modestly because the baseline hot path is bound by the ordered-map and intrusive-list ops, not allocation count. The two recent micro-opts (try_emplace, load-factor 0.25) are kept as a labeled v0.2.2 sub-analysis with their perf-report evidence. before.svg is now the v0.1.0 flamegraph, after.svg v0.2.2; both render with ZERO [unknown] and the libc malloc internals resolved. README and perf-stat.txt updated to the v0.1.0 baseline; all em-dash-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Visual diagrams (all em-dash-free, all special-char labels quoted): - matching_rules: order matching decision flow (limit/market, cross/rest, IOC) - binary_protocol: frame decode + reject pipeline - persistence: append -> app buffer -> page cache -> disk durability layers - ocaml_verifier: C++ vs OCaml differential + shrink pipeline - concurrency_model: SPSC pipeline (input -> engine -> publisher) with backpressure - memory_ordering: producer/consumer happens-before (release synchronizes-with acquire) - socket_gateway: TcpServer accept loop (transient retry, fd-exhaustion backoff, cap shedding) Docs that already had mermaid (architecture, differential_testing, replay_and_recovery, property_testing, benchmarking, README) are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… to -35% Adversarial self-review found a measurement bug in the allocation counter: the global operator-new override only intercepted operator new(size_t) and the array form, missing operator new(size_t, align_val_t). v0.2.2's storage makes ~1.56 OVER-ALIGNED allocations/order (v0.1.0 makes none), so they were uncounted and allocs/order was reported as 1.108 when the true figure is 2.670. The headline "-73% allocations since v0.1.0" was therefore wrong; the real cut is -34.8%. Fixes: - Override every operator new/delete variant (plain + aligned) so the count is complete. The aligned override adds a little work per allocation, which would perturb cycle/throughput numbers, so allocation counting is now compile-time opt-in (QSL_PERFEVAL_COUNT_ALLOCS) behind a second CMake target, qsl-perfeval-allocs. The default qsl-perfeval leaves the allocator untouched (pure performance) and prints allocs_per_order=n/a. - Re-measured both versions cleanly: performance from qsl-perfeval (no instrumentation), allocations from qsl-perfeval-allocs. Frequency-independent counts are the load-bearing metrics (wall-clock throughput is schedutil-noisy). Corrected cumulative v0.1.0 -> v0.2.2 (baseline storage): allocations/order 4.094 -> 2.670 (-34.8%) cycles/order 310.7 -> 289.5 (-6.8%) instructions/order 1215 -> 1157 (-4.7%); IPC 3.91 -> 4.00 branch-miss rate 2.01% -> 1.68% (-16.3% relative) p50/p99 latency 83/209 ns both (unchanged) cache-miss unavailable (Apple Silicon PMU; #90) PERFORMANCE.md, perf-stat.txt, README updated and document the earlier mistake openly. make check/asan 272/272; CodeScene clean; all em-dash-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dpdk_environment.txt and nic_offload_environment.txt carry several docs (CLAUDE.md, AGENTS.md, MILESTONES.md, results/README.md) in their digest scope; regenerated so they stay Dirty inputs: no after the em-dash purge and edits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 41 minutes and 6 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR updates the benchmark harness and validation paths, refreshes performance evidence and generated result files, and reformats repository roadmap, policy, and technical documentation across Markdown, comments, and helper scripts. ChangesRuntime and validation
Performance evidence and artifacts
Repository guidance and lifecycle docs
Technical docs and commentary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/recruiting_notes.md (2)
95-102: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the actual OCaml module path.
ocaml/lib/replay_engine.mllooks stale here; the roadmap and verifier layout in this PR point toocaml/lib/replay.mlinstead. Fixing the path avoids sending recruiters to a file that doesn’t exist.Suggested fix
- typed/immutable OCaml engine (`ocaml/lib/replay_engine.ml`) re-derives the final book state + typed/immutable OCaml engine (`ocaml/lib/replay.ml`) re-derives the final book state🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/recruiting_notes.md` around lines 95 - 102, The OCaml path referenced in the recruiting notes is stale and should match the actual module used by the verifier. Update the description in the relevant markdown section to point from the old replay_engine symbol path to the current OCaml replay module path, and make sure the surrounding explanation still refers to the independent replay implementation and snapshot checks without mentioning the obsolete file name.
57-67: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReconcile these benchmark numbers with the canonical artifact.
These figures no longer match the repo’s canonical benchmark summary, so the résumé bullets and live-state docs will tell different stories. Please regenerate them from the same measured output before keeping them in a recruiting-facing file.
Suggested fix
-- matching-engine flow ~98 ns/command (~10.2M commands/sec) -- order-book add/modify/cancel ~87 ns/op -- protocol `NewOrder` encode+decode ~16 ns/op -- gateway session crossing-fill round-trip ~110 ns/op -- replay from command log ~110 ns/command +- matching-engine flow ~91 ns/command +- order-book add/modify/cancel ~90 ns/op +- protocol `NewOrder` encode+decode ~16 ns/op +- gateway session crossing-fill round-trip ~102 ns/op +- replay from command log ~101 ns/command🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/recruiting_notes.md` around lines 57 - 67, The benchmark bullets are out of sync with the canonical artifact, so update the recruiting note to match the repo’s current measured summary. Regenerate the figures from the same source used by the canonical benchmark output (the `results/latest.txt`-backed summary) and replace the existing measurements in the benchmark section so the recruiting-facing doc and live benchmark state stay consistent. Reference the benchmark section in `docs/recruiting_notes.md` and the measured output source when updating the listed numbers.
🧹 Nitpick comments (1)
tests/CMakeLists.txt (1)
117-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSmoke-test
qsl-perfeval-allocsas well.This PR’s new behavior is behind
QSL_PERFEVAL_COUNT_ALLOCS, butmake checkstill exercises onlyqsl-perfeval. A tinyqsl-perfeval-allocsrun that assertsallocs_per_orderis numeric would catch wiring regressions betweenCMakeLists.txtandapps/qsl-perfeval/main.cpp.Suggested addition
add_test(NAME qsl_perfeval_smoke COMMAND qsl-perfeval 5000 --latency) set_tests_properties(qsl_perfeval_smoke PROPERTIES PASS_REGULAR_EXPRESSION "perfeval: latency_ns") # Malformed order counts must be rejected, not truncated or wrapped. add_test(NAME qsl_perfeval_invalid_count_fails COMMAND ${CMAKE_COMMAND} -D "PROGRAM=$<TARGET_FILE:qsl-perfeval>" -D "ARGS=123abc" -D "EXPECT_OUTPUT=usage:" -P "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") + +add_test(NAME qsl_perfeval_allocs_smoke COMMAND qsl-perfeval-allocs 5000) +set_tests_properties(qsl_perfeval_allocs_smoke PROPERTIES PASS_REGULAR_EXPRESSION + "allocs_per_order=[0-9]")Based on learnings, "Write tests beside deterministic logic."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/CMakeLists.txt` around lines 117 - 126, Add a smoke test for the qsl-perfeval-allocs binary alongside qsl_perfeval_smoke in the CMake test list so make check covers the QSL_PERFEVAL_COUNT_ALLOCS path exercised by apps/qsl-perfeval/main.cpp. Reuse the existing qsl-perfeval pattern with a tiny run and a PASS_REGULAR_EXPRESSION that verifies allocs_per_order is reported as numeric, so wiring regressions between CMakeLists.txt and the main app are caught.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/qsl-perfeval/main.cpp`:
- Around line 283-296: The argument parsing in main currently accepts multiple
order-count values and lets the last one win, which conflicts with the single
optional [orders>0] usage. Update the loop that handles argv parsing so that
after parse_orders_arg succeeds and orders is already set, the program rejects
any additional non-flag order-count arguments with the same usage/error path
instead of overwriting the existing value. Keep the --latency handling intact
and use the existing orders variable in main to detect duplicates.
In `@docs/concurrency_model.md`:
- Around line 191-195: The ownership matrix table has a malformed Input row
caused by an extra cell boundary fragment, so fix the markdown in the table
around the Input/Engine/Publisher rows to restore valid column alignment and
readable rendering. Update the table structure in the concurrency model docs so
the Input row cleanly maps to its Thread owns exclusively, Consumes, and
Produces cells without the stray separator, keeping the rest of the table
consistent.
In `@docs/persistence.md`:
- Around line 17-25: The Mermaid diagram currently labels the final disk node in
a way that implies an absolute guarantee; update the wording in the persistence
diagram to reflect the best-effort durability contract described later in the
document. Adjust the label on the `disk` node in the `EventLogWriter::append`
flowchart so it no longer says “Durable, survives power loss,” and use softer
language consistent with the `fsync`/`F_FULLFSYNC` discussion.
In `@docs/release_readiness.md`:
- Around line 80-101: The release readiness paragraph has a markdown formatting
issue where the `#32` reference is left at the start of a wrapped line, which
can be read as a heading token. Update the `Outcome` text in
`release_readiness.md` so the `#32` reference stays inline with the surrounding
sentence, either by reflowing the sentence or moving the reference onto the
previous line while preserving the same meaning.
---
Outside diff comments:
In `@docs/recruiting_notes.md`:
- Around line 95-102: The OCaml path referenced in the recruiting notes is stale
and should match the actual module used by the verifier. Update the description
in the relevant markdown section to point from the old replay_engine symbol path
to the current OCaml replay module path, and make sure the surrounding
explanation still refers to the independent replay implementation and snapshot
checks without mentioning the obsolete file name.
- Around line 57-67: The benchmark bullets are out of sync with the canonical
artifact, so update the recruiting note to match the repo’s current measured
summary. Regenerate the figures from the same source used by the canonical
benchmark output (the `results/latest.txt`-backed summary) and replace the
existing measurements in the benchmark section so the recruiting-facing doc and
live benchmark state stay consistent. Reference the benchmark section in
`docs/recruiting_notes.md` and the measured output source when updating the
listed numbers.
---
Nitpick comments:
In `@tests/CMakeLists.txt`:
- Around line 117-126: Add a smoke test for the qsl-perfeval-allocs binary
alongside qsl_perfeval_smoke in the CMake test list so make check covers the
QSL_PERFEVAL_COUNT_ALLOCS path exercised by apps/qsl-perfeval/main.cpp. Reuse
the existing qsl-perfeval pattern with a tiny run and a PASS_REGULAR_EXPRESSION
that verifies allocs_per_order is reported as numeric, so wiring regressions
between CMakeLists.txt and the main app are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6effbdd-93f8-4c5f-b597-2cee234200f8
⛔ Files ignored due to path filters (3)
docs/performance/after.svgis excluded by!**/*.svgdocs/performance/before.svgis excluded by!**/*.svgresults/flamegraph.svgis excluded by!**/*.svg
📒 Files selected for processing (76)
.github/ISSUE_TEMPLATE/review_request.md.github/pull_request_template.md.gitignoreAGENTS.mdCHANGELOG.mdCLAUDE.mdCMakeLists.txtCONTRIBUTING.mdHANDOFF.mdMILESTONES.mdPERFORMANCE.mdPROGRESS.mdREADME.mdSECURITY.mdapps/qsl-bench/main.cppapps/qsl-perfeval/main.cppcmake/Sanitizers.cmakedata/synthetic/README.mddocs/adr/0002-independent-ocaml-oracle.mddocs/adr/0003-golden-fixture-regeneration.mddocs/adr/0004-deterministic-shrinker.mddocs/adr/0007-constrained-perf-artifacts-are-partial-evidence.mddocs/adr/0008-socket-evidence-loopback-constrained-epoll-deferred.mddocs/adr/0011-event-log-durability-modes-and-tail-repair.mddocs/allocator_experiment.mddocs/architecture.mddocs/benchmarking.mddocs/binary_protocol.mddocs/concurrency_model.mddocs/differential_testing.mddocs/fix_protocol.mddocs/invariants.mddocs/linux_performance.mddocs/matching_rules.mddocs/memory_ordering.mddocs/ocaml_verifier.mddocs/perf_analysis.mddocs/performance/perf-stat.txtdocs/persistence.mddocs/property_testing.mddocs/recruiting_notes.mddocs/release_readiness.mddocs/replay_and_recovery.mddocs/review_feedback.mddocs/review_request.mddocs/socket_gateway.mddocs/socket_hardening.mddocs/socket_profiling.mdinclude/qsl/concurrency/pipeline.hppinclude/qsl/concurrency/spsc_ring.hppinclude/qsl/feed/sequence_tracker.hppinclude/qsl/feed/udp_feed.hppinclude/qsl/gateway/tcp_server.hppinclude/qsl/protocol/endian.hppinclude/qsl/protocol/fix.hppocaml/lib/diff_report.mlocaml/test/test_mutation.mlregressions/README.mdresults/README.mdresults/dpdk_environment.txtresults/flamegraph.txtresults/nic_offload_environment.txtscripts/determinism_check.shscripts/divergence_demo.shscripts/flamegraph.pyscripts/flamegraph.shsrc/engine/contiguous_store.hppsrc/engine/order_book.cppsrc/gateway/epoll_server.cppsrc/gateway/tcp_server.cppsrc/replay/fixture.cpptests/CMakeLists.txttests/concurrency/test_pipeline.cpptests/shell/test_flamegraph.shtests/unit/test_fix_protocol.cpptests/unit/test_shrink.cpp
…e broke) CodeRabbit flagged four issues on #150; all real, all fixed: - qsl-perfeval: `qsl-perfeval 1000 2000` silently ran with 2000. Reject a second order count as ambiguous. Extracted parse_args() so main stays under the cyclomatic-complexity threshold (CodeScene). - concurrency_model.md + fix_protocol.md: the em-dash purge turned an em-dash table cell into ", ", producing a malformed `|, |` row. Restored the cells ("nothing" consumed / "(none)" internal field). Repo-wide scan confirms these were the only two corrupted table cells. - persistence.md: the new durability mermaid node said "Durable, survives power loss", overclaiming vs the doc's best-effort fsync contract. Softened. - release_readiness.md: a wrapped line started with "#32" (markdownlint heading false-positive); kept the reference inline. make check 272/272; CodeScene clean; 0 em/en dashes repo-wide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Our agent can fix these. Install it.
Gates Passed
Absence of Expected Change Pattern
- quant-systems-lab/src/replay/fixture.cpp is usually changed with: quant-systems-lab/apps/qsl-export-stream/main.cpp
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
div0rce
added a commit
that referenced
this pull request
Jun 25, 2026
* chore(release): cut v0.2.2 Finalize the v0.2.2 changelog entry to include this session's documentation overhaul (#147), performance-evidence report (#148), README rebuild (#149), and the bug/style/mermaid sweep (#150) on top of the post-v0.2.1 hardening + perf wave (#135-#146). Fix the test count (272/272) and flip the v0.2.2 resume/release anchors (PROGRESS.md, release_readiness.md) from "in preparation" to released. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: reconcile test count to 272/272 across release records (CodeRabbit) CodeRabbit flagged docs/release_readiness.md still showing 270/270 while CHANGELOG/PROGRESS said 272/272. The two perfeval tests added this session took the count 270 -> 272, so the current-state and verification claims were stale. Updated all current-state references (release_readiness verification table, PROGRESS.md status + both summary blocks, the CLAUDE.md/AGENTS.md roadmap memory kept in sync, HANDOFF.md), and flipped the v0.2.2 "being cut / next action" phrasing to released. The one remaining 270/270 is a dated entry under PROGRESS.md "Decision log additions" (a correct historical snapshot). All em-dash-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A broad correctness + docs pass driven by the goal "extinguish all bugs (incl. ignored CodeRabbit/Codex comments), make everything current and em-dash-free, add mermaid everywhere."
Bugs fixed (the ignored CodeRabbit findings + one found by self-review)
profile_seconds_from_args:!(seconds > 0.0)letinf/nanfromstrtodthrough intoduration_cast(UB). Now requiresstd::isfiniteand clamps.transient_accept_errno: omittedENOPROTOOPT/EOPNOTSUPPthat Linuxaccept(2)returns and the epoll path already retries (wrongly fatal). Set now matches.parse_ordersaccepted123abc/-1viastrtoull. All fixed (eng.containsfor resting tracking,from_charsvalidation + usage error, corrected percentile index). Test added.operator newoverride missedoperator new(size_t, align_val_t), undercounting v0.2.2's ~1.56 over-aligned allocs/order. The earlier "-73% allocations" claim was wrong; the true cut is -34.8%. Counting is now complete and compile-time opt-in (qsl-perfeval-allocs) so it doesn't perturb the cycle numbers.Flamegraphs: zero
[unknown], every frame identifiedThe
[unknown]frames were identified (not hidden): fp-unwinding artifacts at the glibc-2.43 malloc boundary (x29 not preserved -> one spurious frame between resolvedoperator newand_mid_memalign), plus the vDSOclock_gettimeleaf. DWARF was verified worse (4477 vs 37 unknowns). All flamegraphs render fully symbolized with the libc malloc internals visible.Performance reframed to v0.1.0 -> v0.2.2 (cumulative, honest)
Same harness ported into a
v0.1.0worktree, measured on the same host. Frequency-independent counts are load-bearing:Cache-miss rate stays unavailable (Apple Silicon PMU, #90).
Docs
Verification
make check/make asan272/272; CodeScene clean; all flamegraphs 0[unknown]; 0 em/en dashes repo-wide; determinism unaffected (no engine logic changed).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation