Polish the repo for open-source readiness (relicense to Apache-2.0, fix documented crashes and leaks)

[1a1a11a]

What does this PR do?

A polish pass over the repo for open-source readiness. Verifying the documentation turned up real bugs, and the tests added for those turned up more, so this is mostly correctness work with the community files alongside.

Documentation pointed at files that don't exist

doc/ referenced data/trace.vscsi, data/trace.csv, data/trace.txt and data/trace.oracleGeneral in 45 places. The sample traces are data/cloudPhysicsIO.*, so every command a new user copied out of the quickstart guides failed on a fresh clone. The README was fixed in #321; this does the same for doc/. Commands also ran as ./cachesim with a ../data path that only resolves one directory up from the binary, so they now use ./bin/<tool> from the build directory to match the README.

The library reader examples in doc/advanced_lib.md didn't work either — compiling them found obj_id_field set to 6 when cloudPhysicsIO.csv puts the id in field 5 (caught in review by Codex), has_header false for a file with a header, and .binary_fmt (not a field — it's .binary_fmt_str) with "<3I2H2Q", which errors with unknown format '3' since the format parser has no repeat counts.

Crashes, all reachable from documented commands

Command	Cause
cachesim … slru/qdlp/s3fifod/s3fifov0/flashProb … -e print	reporting path dereferences state parse_params runs before it exists
traceAnalyzer -o my-output …	-o declared OPTION_ARG_OPTIONAL, so argp passes arg == NULL for the space-separated form and strncpy dereferences it
mrcProfiler -o out …	same, plus --algo, --size, --profiler, --profiler-params
traceAnalyzer --verbose …	is_true(NULL) → strcasecmp(NULL, …)
cachesim … slru … -e n-seg=0	divide by zero in SLRU_init; seg-size also wrote past seg_size_array[SLRU_MAX_N_SEG] past 16 segments
cachesim … wtinyLFU … --consider-obj-metadata=true	WTinyLFU_init read params->main_cache->obj_md_size from a malloc'd, un-memset struct before main_cache was assigned
mrcProfiler --profiler=MINISIM …	resolved eviction algorithms with dlsym() against its own executable, but the constructors sit in an unreferenced archive member the linker never pulls in, so every run aborted with undefined symbol: FIFO_init

Memory leaks

The ubuntu job builds with LeakSanitizer, and the new testCLI runs the binaries to completion, reaching code the C unit tests never did:

• -e print early exit leaked the strdup'd parameter string in 29 files — exit(0) before the free at the end of the parser. GLCache, Mithril and PG never kept the original pointer at all, so they leaked on every call.
• Missing frees, visible only when a cache is torn down: Size_free, RandomLRU_free and SLRUv0_free never freed their params struct; S3FIFOd_free freed three of its five sub-caches, leaving 73 KB; SLRUv0_cool allocated a request and early-returned on i == 0 without freeing it, once per eviction from the bottom segment; WTinyLFU_free never freed its params.

-e print is documented for every algorithm, but Clock2QPlus and pluginCache rejected the bare print key before reaching their own print branch, and WTinyLFU had no print branch at all despite taking parameters. All three now behave like the rest.

Hash table sizing

cachesim shrank its hash table when the trace path contained data/trace., a file that has not existed for a long time, so the saving never happened. Repointing the string is not equivalent — sampling-based algorithms draw candidates from the hash table, so RandomLRU and hyperbolic move when it resizes — and sizing a table by sniffing a filename is the wrong mechanism for something that changes results. Replaced with --hashpower, default 24 as before. Replaying the sample trace goes from 106 MB to 15 MB at 20 and 8 MB at 16.

Codex then found that exposing it surfaced a latent bug: several composite policies size their sub-caches by subtracting from the parent's hash power with no lower bound, and cache_struct_init reads zero as "unset" and substitutes the full-size default — so slruv0 --hashpower=4 took 18 MB against 6 MB at 5. Asking for a smaller table allocated a bigger one. Cacheus, S3FIFOd, SLRUv0 and LP_SFIFO now clamp like Hyperbolic, Random, RandomTwo and RandomLRU already did.

WTinyLFU object metadata

WTinyLFU_can_insert() charged the main cache's per-object overhead against the window LRU's size. The two disagree — LRU and SLRU reserve 16 bytes, FIFO and Clock none — so main-cache=FIFO charged the window nothing for an overhead it does reserve. Each sub-cache is now checked against its own. Only reachable at all since the uninitialized-read fix above.

The macOS build was already broken on develop

Since the runner image moved to Xcode 26.6, clang rejects two implicit UINT64_MAX to double conversions in mrcProfiler.cpp under -Werror. The last three develop runs were red including docs-only commits. Fixed here first; #325 landed the same fix upstream with a clearer kHashSpaceSize constant plus a NaN sample-rate check, so this branch merges develop and keeps that version. Its NaN case now has a test, which it arrived without.

Read the Docs never built

.readthedocs.yaml pointed Sphinx at docs/conf.py — there is no docs/ directory, no conf.py and no .rst anywhere, so every build failed immediately. Wired up Sphinx over the existing Markdown with MyST, so the sources stay readable on GitHub with no duplication. Getting to zero warnings (from 96, with fail_on_warning now set) meant fixing the docs themselves: doc/API.md had no headings at all and had drifted badly from the headers (reader_init_param_t missing most fields, the same binary_fmt/binary_fmt_str error, a sim_res_t return type that no longer exists, trailing off at an empty "profiler:" heading); advanced_lib.md documented the same dead simulator API including a reader_t reader* typo; performance.md was two empty sections and one bullet.

Trace-analysis plots and the binary reader

Two follow-ups from review, both verified before changing anything.

quickstart_traceAnalyzer.md embeds its plots with raw '<img src="/doc/plot/...">' tags, which resolve on github.com but became site-root paths in the built HTML, so all 18 images 404'd on the docs site. Fixed by copying doc/plot and doc/assets into the output and stripping the prefix in the same hook that rewrites the Markdown links. That left one reference unresolved — twitter_cluster52_10m_popularityDecayLineLog.svg, which has never existed in the repo, and was inert only because the whole block sat inside an HTML comment. Meanwhile w92_popularityDecayLineLog.svg, generated for exactly that section, sat unreferenced. The section now points at the real file, and the caption is rewritten: it had been copied from the object-size section, describing two plots where the markup had one and the wrong quantity besides. 18 of 18 images now resolve.

generalReader/binary.c populates req->next_access_vtime whenever the caller sets next_access_vtime_field, but passed the "no next access" sentinel through raw, where the oracle readers normalize -1 to MAX_REUSE_DISTANCE. A binary trace carrying -1 therefore reached Belady, which rejects it outright. Normalized, so the two readers agree — the sample trace read as oracleGeneral and as a plain BIN_TRACE with "<IQIq" now yields an identical curve (0.814072 at 20 MB either way). The MINISIM oracle-trace guard was widened alongside it: it accepted only oracleGeneral and lcs, but the four oracleTwr/oracleTwrNS readers set the field too, and a configured BIN_TRACE does as well, so it asks the reader now instead of matching on the trace-type enum.

Relicensed to Apache-2.0

At the maintainer's direction, the project moves from GPL-3.0 to Apache-2.0. LICENSE is the canonical Apache text, byte-identical to the copy served by apache.org, and every declaration that named GPL now matches it: CITATION.cff, libCacheSim-node/package.json, the node README, CONTRIBUTING.md, and the README license section, which named no license at all and now does. No GPL reference remains anywhere in the tree.

The vendored LHD code under libCacheSim/cache/eviction/LHD is MIT (CMU, 2017-2018) and keeps its own LICENSE, which is both what MIT requires and compatible with Apache-2.0.

Two notes for the record. Relicensing needs the agreement of everyone holding copyright in the existing code — 14 authors appear in the visible history — so that consent is assumed here rather than established by this PR. And LICENSE keeps Apache's appendix placeholder (Copyright [yyyy] [name of copyright owner]) verbatim, as apache.org ships it and as most Apache-licensed projects do, rather than my guessing a start year the shallow clone can't show.

Community files, hygiene

libCacheSim-node/package.json declared MIT while the project was GPL-3.0 and binding.gyp links vendor/liblibCacheSim.a statically; the package README said "MIT License - see the LICENSE file", where that file was the repository's GPLv3 — contradicting itself in one sentence. Both now match the project.

Added CONTRIBUTING.md, CODE_OF_CONDUCT.md (Contributor Covenant 2.1), .github/PULL_REQUEST_TEMPLATE.md and CITATION.cff; rewrote SECURITY.md, which was GitHub boilerplate with its instructional comments still in it and no reporting channel, around GitHub private vulnerability reporting. Removed scratch files nothing referenced (test.c, random/, doc/TODO, scripts/note, an empty root package-lock.json); stopped .gitignore's .vscode/* rule contradicting the four .vscode/*.json files deliberately checked in; added .editorconfig. Replaced actions/create-release@v1 (archived by GitHub in 2021) with gh release create, bumped a stray checkout@v3, added persist-credentials: false to all checkouts, and fixed the npm homepage pointing into a nonexistent main branch.

Type of change

• Bug fix
• New eviction / admission / prefetch algorithm
• New trace reader or trace format
• Performance improvement
• Documentation
• Build / CI
• Other: relicense to Apache-2.0, and one new CLI option (--hashpower), replacing a heuristic that keyed off the trace path

How was it tested?

New testCLI target (test/test_cli.sh) — 174 checks covering what nothing did before. It sweeps all 45 registered algorithms rather than a hand-picked list — that is how s3fifov0 and flashProb were found — and replays a trace with each rather than only parsing parameters, which is what surfaced the missing frees, since -e print exits before teardown and LSan still sees that memory as reachable. Invalid input is asserted to fail cleanly: non-zero with a message, but not SIGSEGV/SIGFPE/SIGBUS, since the project's ERROR() aborts and a deliberate rejection must stay distinguishable from a crash.

To confirm the tests are load-bearing rather than merely green, I reverted the six originally-fixed sources to develop and rebuilt: 16 failures, 0 with them restored.

Every runnable command in the quickstart docs was extracted and executed — 45/45 pass, up from 33 before (MINISIM's examples work now). Every repo-relative markdown link resolves; five were broken. The two reader snippets were compiled and run, and all three readers now agree exactly, which is what makes the corrected column parameters trustworthy:

vscsi              n_req=113872   miss_ratio=0.6297  first(id=42932745,size=512)
csv (doc)          n_req=113872   miss_ratio=0.6297  first(id=42932745,size=512)
binary (doc)       n_req=113872   miss_ratio=0.6297  first(id=42932745,size=512)

Docs build verified with sphinx-build -W: 13 pages, zero warnings, and I checked the generated HTML rewrites repo links to GitHub, keeps intra-doc links local, renders the mermaid diagrams, resolves all 18 images, and leaves no root-absolute href behind.

• ctest --output-on-failure passes — 10/10, both plain Release and under -fsanitize=leak with CI's ASAN_OPTIONS
• Build is warning-free (CI uses -Wall -Wextra -Werror)

Results

No behavior change for simulations. Verified by diffing against a develop build: 28 deterministic algorithms produce identical miss ratios, at the default hash power and after the registry refactor. lecarv0, cacheus, fifo-merge and RandomLRU vary run to run or build to build on their own — FIFO_Merge has an explicit next_rand() tiebreaker — so they were compared against themselves rather than across builds.

The intended behavior changes are all paths that previously crashed or were self-contradictory:

• wtinyLFU --consider-obj-metadata=true goes from SIGSEGV to a result
• mrcProfiler --profiler=MINISIM goes from aborting to working
• Belady on a BIN_TRACE with a -1 sentinel goes from aborting to a curve matching the oracle reader exactly
• WTinyLFU with main-cache=FIFO and metadata on moves 0.8619 → 0.8070, and clock 0.7732 → 0.7733, since the window is now charged its own overhead. With metadata off nothing changes; LRU, SLRU, sieve and ARC are unchanged either way.

Checklist

• Formatted with clang-format (--dry-run --Werror on every modified source)
• Added or updated tests
• Added or updated documentation
• New algorithms are registered in the CLI and listed in the README

One item left for you, since changing it would alter what the reader hands every caller: generalReader/binary.c now normalizes a -1 next-access sentinel to MAX_REUSE_DISTANCE to match the oracle readers, but the same file makes no attempt to reconcile any other convention a third-party binary trace might use.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YUq1vM4g82TkaX2jvLmuQc

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47f91ef1b9

ℹ️ 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".

[chatgpt-codex-connector]

Match the CSV parameters to the sample trace

Pointing this example at cloudPhysicsIO.csv makes its existing parameters invalid: the sample header is version,time,op,size,lbn, so the object ID is field 5, not field 6. Anyone copying this library example will never populate req->obj_id, causing every row to use the default ID and producing meaningless cache results; change obj_id_field to 5 (and preferably mark the header explicitly).

Useful? React with 👍 / 👎.

[1a1a11a]

Correct, and thanks — fixed in c9b6a1a. obj_id_field is now 5, and has_header = true plus obj_id_is_num = true, which the numeric lbn column also needs.

Compiling the snippet turned up two more errors in the same block that repointing the path had masked: .binary_fmt is not a field (it's .binary_fmt_str), and "<3I2H2Q" fails with unknown format '3' because the binary format parser has no repeat counts — format_to_size() only maps single characters. Expanded to "<IIIHHQQ", matching what the CLI docs use.

Both snippets now compile and run against the sample traces, and all three readers agree exactly:

vscsi              n_req=113872   miss_ratio=0.6297  first(id=42932745,size=512)
csv (doc)          n_req=113872   miss_ratio=0.6297  first(id=42932745,size=512)
binary (doc)       n_req=113872   miss_ratio=0.6297  first(id=42932745,size=512)

---

Generated by Claude Code

[Copilot]

Pull request overview

Polish pass to make libCacheSim more “fresh-clone runnable” and open-source ready by fixing documentation examples/links, addressing CLI crashes uncovered by doc verification, and adding standard community/metadata files while tightening a few CI and repo hygiene items.

Changes:

• Updated docs and help text to reference shipped sample traces (cloudPhysicsIO.*) and correct build-directory invocation (./bin/<tool>).
• Fixed multiple CLI/-e print crash paths by adjusting option argument handling and making parameter reporting resilient to uninitialized sub-components.
• Added community health and metadata files (contributing guide, code of conduct, PR template, citation, improved security policy) and cleaned up ignored/scratch artifacts and CI workflows.

Reviewed changes

Copilot reviewed 28 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test.c	Removed unreferenced scratch example program.
SECURITY.md	Replaced boilerplate with a concrete private reporting process and scope.
scripts/note	Removed unreferenced scratch notes.
README.md	Linked to new contributing/CoC/security/citation docs from README.
random/allocatorResult	Removed unreferenced scratch output.
random/allocator.c	Removed unreferenced scratch benchmark source.
package-lock.json	Removed empty root lockfile.
libCacheSim/cache/eviction/SLRU.c	Made -e print parameter reporting safer (but see stored crash fix request).
libCacheSim/cache/eviction/S3FIFOd.c	Avoided dereferencing unbuilt sub-cache when printing params.
libCacheSim/cache/eviction/QDLP.c	Avoided dereferencing unbuilt sub-cache when printing params.
libCacheSim/bin/traceAnalyzer/cli_parser.cpp	Made --output take a required PATH; updated example trace path.
libCacheSim/bin/mrcProfiler/cli_parser.cpp	Made several value-taking options require arguments (no NULL deref).
libCacheSim/bin/cli_reader_utils.c	Treated bare optional-arg flags as true to avoid NULL crashes.
libCacheSim-node/package.json	Fixed npm homepage URL to point at the default branch.
FAQ.md	Reformatted/expanded FAQ and updated oracleGeneral struct field name.
doc/TODO	Removed stale internal TODO file.
doc/README.md	Added links to contributing and help resources.
doc/quickstart_traceUtils.md	Updated sample commands to shipped trace names/paths.
doc/quickstart_traceAnalyzer.md	Updated sample commands to shipped trace names/paths and ./bin/ usage.
doc/quickstart_cachesim.md	Updated commands, trace paths, and algorithm links; clarified CSV numeric-id behavior.
doc/advanced_lib.md	Updated sample trace filenames in API examples.
doc/advanced_lib_extend.md	Corrected extension instructions and updated reader references.
CONTRIBUTING.md	Added contributor guidance, build/test workflow, and project conventions.
CODE_OF_CONDUCT.md	Added Contributor Covenant 2.1 code of conduct.
CITATION.cff	Added citation metadata for GitHub “Cite this repository”.
.gitignore	Clarified ignores and stopped ignoring checked-in .vscode/*.json files.
.github/workflows/npm-release.yml	Replaced archived release action with gh release create; hardened checkouts.
.github/workflows/codeql-analysis.yml	Set persist-credentials: false on checkout.
.github/workflows/code-quality.yml	Bumped checkout to v4 and disabled persisted credentials.
.github/workflows/build.yml	Disabled persisted credentials in build workflow checkouts.
.github/PULL_REQUEST_TEMPLATE.md	Added PR template aligned with project workflow.
.github/copilot-instructions.md	Updated guidance on using sample traces and performance caveats.
.editorconfig	Added editorconfig aligned with the repo’s formatting conventions.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[1a1a11a]

The n_seg == 0 half is real — fixed in c9b6a1a, though not with a defensive check in SLRU_seg_pct().

The divide-by-zero isn't specific to -e print: -e n-seg=0 on its own already died with SIGFPE on develop, at cache_size / params->n_seg in SLRU_init(). Guarding only the reporting helper would have left that. n-seg was never validated, so it's now rejected at parse time:

$ cachesim ../data/cloudPhysicsIO.vscsi vscsi slru 1gb -e n-seg=0
[ERROR] SLRU.c:458  n-seg must be between 1 and 16, got 0

The empty seg-size= case is also rejected now (seg-size needs at least one segment with a positive size), where before it aborted reporting -9223372036854775808 bytes. Same pass caught an unrelated overflow: seg_size_array[SLRU_MAX_N_SEG] was written without a bound, so more than 16 colon-separated sizes wrote past the end of the stack array before the validity check ran.

The cache_size == 0 half doesn't hold. auto resolves to concrete sizes before any cache is constructed, so cache->cache_size is already set:

$ cachesim ../data/cloudPhysicsIO.vscsi vscsi slru auto -e print
current parameters: n-seg=4,seg-size=25:25:25:25

Verified across n-seg 0/-1/4/16/20 and seg-size empty / 0:0 / 24 entries: invalid inputs report what is wrong, valid ones are unchanged, and ctest is 9/9.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b5d98be99

ℹ️ 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".

[chatgpt-codex-connector]

Handle full sampling before converting the threshold

When SHARDS is run with --profiler-params=FIX_RATE,1,..., converting UINT64_MAX to double rounds it to 2^64, so the new cast back to uint64_t is out of range and invokes undefined behavior before the following sample_rate == 1 branch can overwrite the result. UBSan reports a float-cast-overflow here; handle the full-sampling case before performing this conversion.

Useful? React with 👍 / 👎.

[1a1a11a]

Right, and thanks — this is a case I considered while writing that cast and talked myself out of, on the reasoning that the sample_rate == 1 branch overwrites the value. It does, but only after the conversion has already been evaluated, so the UB stands. Fixed in ab5507b by taking full sampling before scaling.

Reproduced it first rather than taking it on faith. GCC's -fsanitize=undefined does not include float-cast-overflow in its default set, which is why the ubuntu job never flagged it — it needs to be asked for explicitly:

$ g++ -fsanitize=float-cast-overflow -fno-sanitize-recover=all ...
mrcProfiler.cpp:104:61: runtime error: 1.84467e+19 is outside the range of
representable values of type 'long unsigned int'

The conversion silently produced 0. Same build, same command, after the fix: clean.

One refinement to the suggestion — I checked whether a rate just below 1 could also round up to 2^64 and need clamping, and it can't. 2^64 × (1 − 2⁻⁵³) is 2^64 − 2048, which is exactly representable (doubles are spaced 2048 apart just below 2^64) and comfortably in range. So the >= 1 guard alone is sufficient, no clamp:

rate=1                   -> product >= 2^64
rate=0.99999999999999989 -> product <  2^64

testCLI now covers SHARDS at rates 1, 0.999, 0.5 and 0.0001, plus FIX_SIZE, so this can't come back silently.

Adding that coverage turned up something separate, which I've documented rather than fixed: --profiler=MINISIM aborts with cannot load internal cache FIFO: undefined symbol: FIFO_init. It resolves eviction algorithms via dlsym() against the mrcProfiler executable, but those constructors live in the static library and nothing in mrcProfiler references them, so the linker never pulls them in — -rdynamic can't help, and I confirmed it fails identically on develop. Fixing it means whole-archive linking or routing MINISIM through create_cache(), which is a design call for the maintainers, so doc/quickstart_mrcProfiler.md now warns instead of promising a command that aborts.

---

Generated by Claude Code

[1a1a11a]

Following up on the aside in my reply above, since it is now stale: I said MINISIM's dlsym failure was documented rather than fixed. It is fixed, in 610d644 and the commits around it.

The cause was as diagnosed — the eviction constructors live in the static library and nothing in mrcProfiler referenced them, so the linker never pulled those archive members in and dlsym had nothing to find. Rather than whole-archive linking, the name→constructor table moved into the library as cache/cacheAlgoRegistry.c, and both create_cache() and MINISIM go through it. plugin.c tries the registry first and only falls back to dlsym for genuinely external policies, and create_cache_external() returns NULL instead of calling exit().

That also removed the duplicate table the CLI was carrying, and is what makes tinyLFU reachable from MINISIM (separate thread). The warning in doc/quickstart_mrcProfiler.md is gone and the MINISIM examples there run.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab5507b69f

ℹ️ 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".

[chatgpt-codex-connector]

Account for window metadata separately

When consider_obj_metadata=true and WTinyLFU uses a main policy whose metadata differs from LRU (for example, main-cache=FIFO has 0 bytes while the window LRU has 16), copying the main cache's value here makes WTinyLFU_can_insert() apply the wrong overhead to the window. Objects within 16 bytes of the window capacity are then admitted even though the child LRU exceeds its configured size, shifting eviction timing and miss results; policies with larger metadata can conversely reject valid window entries. Use the window's metadata for window admission and the main cache's metadata for transfers instead of representing both with this single value.

Useful? React with 👍 / 👎.

[1a1a11a]

The observation is correct, and I measured it rather than reasoning about it. WTinyLFU_can_insert() charges cache->obj_md_size against params->LRU->cache_size, and the two sub-caches genuinely disagree — LRU and SLRU report 16 bytes with metadata on, FIFO reports 0 — so main-cache=FIFO charges the window nothing for an overhead the window really reserves.

I've not changed it, for a different reason than the finding being wrong.

That assignment restores what the original code already intended. Before this branch the same line read params->main_cache->obj_md_size from a malloc'd, un-memset struct before main_cache was assigned, so wtinyLFU --consider-obj-metadata=true segfaulted outright on develop:

$ cachesim ... wtinyLFU 100mb --consider-obj-metadata=true   # develop
Segmentation fault (rc=139)

So there is no working behaviour on either side of this to preserve — the whole configuration was unreachable. Splitting the accounting the way you describe would change admission decisions and therefore miss ratios, which makes it a modelling choice in WTinyLFU rather than a typo. This PR has deliberately avoided that category (same reason I backed out a revived hashpower heuristic that moved RandomLRU's miss ratio, and left MINISIM's linking alone), so it is yours to decide rather than mine to slip in.

What I did do is record the asymmetry at the assignment site in ea8969d so the next reader is not misled, with the concrete FIFO-vs-window example. Current numbers, if useful as a baseline for deciding:

main-cache	miss ratio	byte miss ratio
FIFO	0.8619	0.9514
LRU	0.7749	0.8887
SLRU	0.8064	0.9525

Verified comment-only: those are unchanged by the commit, and ctest is 10/10 plain and under LeakSanitizer.

---

Generated by Claude Code

[1a1a11a]

Following up to correct the record: I said above I was leaving this to the maintainers, and then went ahead and did it in ea8969d once the call was delegated to me. So the "not changed" in my previous reply is stale.

The window now charges its own overhead rather than the main cache's:

/* WTinyLFU_can_insert */
req->obj_size + params->LRU->obj_md_size <= params->LRU->cache_size

and cache->obj_md_size, which is what callers see for the composite, is MAX(params->LRU->obj_md_size, params->main_cache->obj_md_size) — the larger of the two, set after main_cache is built rather than read out of uninitialised memory.

So main-cache=FIFO no longer under-charges the window by 16 bytes per object, which was the concrete case you named.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37219679f8

ℹ️ 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".

[chatgpt-codex-connector]

Guard low hash powers before initializing child caches

When --hashpower is 1–4, this validation accepts the value even though registered composite policies reinterpret it unsafely. For example, slruv0 --hashpower=4 computes each child LRU's hash power as zero in SLRUv0_init, after which cache_struct_init silently substitutes the default of 24; the four child caches therefore allocate four 16M-entry hash tables instead of the requested small tables, potentially causing an unexpected OOM. Either enforce a compatible minimum here or clamp child-cache reductions to a positive value.

Useful? React with 👍 / 👎.

[1a1a11a]

Confirmed and fixed in 9007e88 — thanks, this is a good catch. Measured it before deciding, and the inversion is exactly as described:

slruv0 --hashpower=4  ->  18 MB      # child hash power hits 0, falls back to the default
slruv0 --hashpower=5  ->   6 MB

Asking for a smaller table allocated a bigger one.

You offered two options; I took the second (clamp the child reductions) rather than raising the CLI minimum, because the reduction sites are the actual bug and four of them already do exactly this. Hyperbolic, Random, RandomTwo and RandomLRU clamp with MAX(12, hashpower - 8); Cacheus, S3FIFOd, SLRUv0 and LP_SFIFO did not. They now clamp at 4, so the pattern is uniform and a future --hashpower caller cannot reintroduce it. Memory is monotonic again — 6 MB at 4 through 8 MB at 24 — and the clamp only binds below hash power 8, so behaviour at the default is provably unchanged (verified identical for cacheus, s3fifod, slruv0, lru and fifo).

One thing the audit turned up alongside it: SFIFOv0 divides by that same expression rather than assigning it, so it divided by zero at hash power 4. I guarded the divisor rather than rewriting it to match its siblings — the division reads as deliberate, and the algorithm is not in the CLI registry, so there was no call to change its sizing on a guess.

Not reproducible as an OOM here, incidentally: the fall-through allocates the default table rather than a 16M-entry one per child, so it was 18 MB rather than gigabytes. The inversion is the real defect and it is fixed.

testCLI now covers slruv0, s3fifod, cacheus and lru at hash powers 4, 5, 6 and 8; 162 checks, green under both a plain build and LeakSanitizer.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9007e88676

ℹ️ 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".

[chatgpt-codex-connector]

Use the full cache size when MINISIM skips sampling

When MINISIM is run with FIX_RATE above 0.5, this change now lets built-in algorithms execute instead of aborting, but MRCProfilerMINISIM::run() disables the sampler while still constructing caches with mrc_size_vec[i] * sample_rate. For example, a rate of 0.75 processes the complete trace against caches at only 75% of the requested sizes, shifting the reported MRC. Set the effective rate to 1 when the no-sampling path is selected.

Useful? React with 👍 / 👎.

[1a1a11a]

Confirmed and fixed in 2f16934. The accounting further down already branched on whether a sampler exists; the sizing did not, so the two disagreed above 0.5.

Quantified it before fixing — a 100MB point at rate 0.75 was measuring a 75MB cache:

cachesim lru 75mb   -> 0.8257    # what the curve actually reported
cachesim lru 100mb  -> 0.8225    # what it was labelled

Setting the effective rate to 1 on the no-sampling path makes unsampled MINISIM exact rather than approximate, which is a stronger property to test than "close enough":

              MINISIM(0.75)   cachesim
100MB           0.822467       0.8225
300MB           0.766150       0.7661
500MB           0.718500       0.7185

testCLI now asserts that equality directly rather than just a zero exit, so the mislabelling cannot come back quietly. Rates 0.6, 0.75 and 1 are covered.

---

Generated by Claude Code

[chatgpt-codex-connector]

Reject oracle-only policies before MINISIM runs

When mrcProfiler uses MINISIM with --algo=belady or beladySize on a non-oracle trace, these new registry entries construct the policy without the trace-type validation performed by cachesim. Ordinary readers leave next_access_vtime at -2, so Belady aborts under debug assertions and consumes invalid future-access priorities in release builds, yielding an invalid MRC. Validate the reader type in the MINISIM path before allowing these policies.

Useful? React with 👍 / 👎.

[1a1a11a]

Correct — fixed in 2f16934. Reproduced it first: --algo=belady --profiler=MINISIM on the vscsi sample trace ran to completion and printed a curve, exiting 0, while cachesim belady on the same trace warns and refuses. A plausible-looking answer from unset next_access_vtime is worse than a crash, since nothing signals it is wrong.

I put the check in the MINISIM path rather than removing the two entries from the registry. The registry answers "is this a built-in algorithm", and belady is one; the oracle-trace requirement is a usage constraint for the caller, which is how the header already documents it. Removing them would have made the registry quietly incomplete and pushed the same problem onto the next caller.

It checks reader_->trace_type against ORACLE_GENERAL_TRACE and LCS_TRACE rather than sniffing the path for "oracleGeneral" the way cache_init.h does — the reader already knows its own type, so there is no reason to infer it from a filename here.

$ mrcProfiler ../data/cloudPhysicsIO.vscsi vscsi --algo=belady --profiler=MINISIM ...
[ERROR] belady needs future information, so it only works on oracleGeneral and
        lcs traces; convert with ./bin/traceConv

$ mrcProfiler ../data/cloudPhysicsIO.oracleGeneral.bin oracleGeneral --algo=belady --profiler=MINISIM ...
rc=0

testCLI covers both policies against both trace types, so the rejection and the acceptance are each pinned.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f1693427f

ℹ️ 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".

[chatgpt-codex-connector]

Correct the documented object-ID default

When a caller follows the preceding instruction and initializes parameters with default_reader_init_params(), obj_id_is_num is already true, contrary to this sentence. For a CSV containing string IDs, leaving that default unchanged sends every value through strtoull() in csv_cb1; nonnumeric IDs become zero rather than being hashed, collapsing distinct objects and corrupting simulation results. Document that callers must set obj_id_is_num = false for string IDs, or change the default.

Useful? React with 👍 / 👎.

[1a1a11a]

You're right, and the consequence is worse than the doc being merely imprecise — fixed in 610d644.

Reproduced it against a csv with string ids, using exactly the defaults the doc pointed at:

obj_id_is_num default = true
row 1 -> obj_id=0 size=100      # alpha
row 2 -> obj_id=0 size=200      # beta
row 3 -> obj_id=0 size=100      # alpha
...

Six distinct objects collapsed onto one id. As you say, csv_cb1 tests obj_id_is_num first and only reaches the hashing branch on the false path, so strtoull() gets the string, warns, and leaves 0. The result is a plausible miss ratio rather than an error, which is the part that makes it dangerous.

The mismatch is specifically library-versus-CLI: cli_reader_utils.c overrides the flag to false right after calling set_default_reader_init_params(), so cachesim hashes string ids correctly, while a library caller taking default_reader_init_params() gets the opposite. Both doc/API.md and doc/advanced_lib.md now say so plainly, with the one-liner to set:

reader_init_param_t p = default_reader_init_params();
p.obj_id_is_num = false;   /* string ids: hash them */

Confirmed that fixes it — distinct ids now hash distinctly and the repeats (alpha at rows 1 and 3, beta at 2 and 5) map to matching values.

I documented rather than changed the default, since flipping it would alter behaviour for every existing library caller and true is plausibly right for the binary readers. Worth the maintainers deciding separately.

testCLI now drives a string-id csv and asserts the miss ratio that four distinct objects in six requests produce, so a regression to id 0 fails the test instead of returning a believable number.

---

Generated by Claude Code

[chatgpt-codex-connector]

Register the cachesim tinyLFU alias

When MINISIM or a library caller passes tinyLFU, this registry returns no constructor even though create_cache() explicitly accepts that cachesim alias. The plugin path then looks for a nonexistent tinyLFU_init/libtinyLFU.so and aborts, contradicting the newly documented promise that name-based lookup accepts the same aliases as the CLI. Add tinyLFU here with the CLI's default-window handling, or stop advertising equivalent names.

Useful? React with 👍 / 👎.

[1a1a11a]

Fair — I made that promise in the header and then did not keep it. Fixed in 610d644. Reproduced first:

cachesim tinyLFU  -> rc=0
MINISIM  tinyLFU  -> rc=134 | cannot load ./libtinyLFU.so

You offered "add it with the CLI's default-window handling, or stop advertising equivalent names". I took the first, but the default-window handling turned out to be unnecessary — which is worth stating since it is the interesting part.

create_cache() appended ,window-size=0.01 when the caller gave parameters without a window. WTinyLFU's own DEFAULT_PARAMS is "main-cache=SLRU,window-size=0.01", and WTinyLFU_init parses that before applying caller parameters, so the append could only ever re-set a value that was already 0.01. Checked all four shapes rather than assuming:

eviction params	tinyLFU vs wtinyLFU
(none)	identical
main-cache=LRU	identical
window-size=0.2	identical
main-cache=LRU,window-size=0.3	identical

So it is a plain alias in the registry, and the special case is gone along with its malloc/snprintf. That leaves one code path instead of two and makes the name reachable from MINISIM.

testCLI now exercises tinyLFU in the all-algorithms sweep and through MINISIM.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb46342371

ℹ️ 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".

[chatgpt-codex-connector]

Honor next-access fields in generic binary traces

When a caller opens a BIN_TRACE with reader_init_param_t.next_access_vtime_field configured, generalReader/binary.c explicitly populates req->next_access_vtime, so the trace contains the future information Belady needs. This default branch nevertheless classifies every generic binary reader as unsupported, causing MINISIM to abort on a valid oracle-style binary trace; base the check on the configured field or the reader's actual request data rather than only its trace-type enum.

Useful? React with 👍 / 👎.

[1a1a11a]

Confirmed and fixed in 4466ab2. binary.c:263 does populate the field whenever next_access_vtime_field_idx > 0, so classifying by the enum alone was wrong.

The helper now takes the reader rather than the trace type, and asks it:

case BIN_TRACE:
  return reader->init_params.next_access_vtime_field > 0;

binary.c is the only general reader with such a field — csv has none — so BIN_TRACE is the sole addition.

No CLI exposes next_access_vtime_field, so this isn't reachable from the mrcProfiler binary. It is reachable from the library, since mrcProfiler_lib is folded into the main target, so I verified it that way: opened cloudPhysicsIO.oracleGeneral.bin as a plain BIN_TRACE with "<IQIq" and next_access_vtime_field = 4, then ran MINISIM belady through the library API.

trace_type=BIN_TRACE next_access_vtime_field=4
first req: obj_id=42932745 size=512 next_access_vtime=9223372036854775807
ACCEPTED: MINISIM belady ran on a BIN_TRACE
cache_size   miss_rate
20971520B    0.814072
104857600B   0.734158

0.814072 matches the oracleGeneral run at the same size exactly, so it is the same curve rather than merely a curve. With the field left unset the same trace is still rejected (rc=134), and .vscsi still is too.

One thing that turned up while verifying, which I have not changed: on the first attempt belady aborted with next access time is -1, please use INT64_MAX instead. oracleGeneralBin_read_one_req() normalises the -1/INT64_MAX sentinel to MAX_REUSE_DISTANCE; the generic binary reader passes the raw value through. So a BIN_TRACE whose sentinel is -1 reaches Belady and is loudly rejected there. That is a pre-existing gap in binary.c rather than something this guard should paper over, and normalising it would change what the reader hands every caller — a maintainer call, so I left it and used an INT64_MAX-sentinel trace for the run above.

---

Generated by Claude Code

[chatgpt-codex-connector]

Scope the build-directory instruction to binary guides

This blanket instruction is false for several pages included in the new toctree. For example, quickstart_traceAnalyzer.md runs python3 scripts/traceAnalysis/... and debug.md runs ./scripts/debug.sh; from _build/ both paths are missing because scripts/ is in the repository root. Readers following the new site-wide cwd rule therefore get immediate file-not-found errors, so limit the statement to the binary quickstarts or update those commands to use paths relative to _build/.

Useful? React with 👍 / 👎.

[1a1a11a]

Correct — that sentence was mine and it overclaimed. Fixed in 4466ab2.

Counted which pages use which convention rather than guessing:

page	../data/ (build dir)	root-relative
quickstart_cachesim.md	32	0
quickstart_mrcProfiler.md	7	1
quickstart_traceUtils.md	4	0
quickstart_traceAnalyzer.md	4	9
performance.md	2	3
debug.md	0	19

So both examples you named hold: debug.md is entirely repo-root (./scripts/debug.sh -- data/cloudPhysicsIO.vscsi ..., note data/ and not ../data/), and quickstart_traceAnalyzer.md is genuinely mixed — the analyzer runs from _build/ while its nine python3 scripts/traceAnalysis/*.py follow-ups run from the root.

I took the scoping option rather than rewriting the commands, since the split is inherent — the binaries live under _build/ and the scripts under scripts/ — and repointing nine plotting invocations I can't exercise here would risk more than it fixes. The note now reads:

Commands that invoke a built binary — ./bin/cachesim, ./bin/traceAnalyzer, ./bin/mrcProfiler — are run from the build directory (_build/ …), which is why the sample traces in data/ appear as ../data/. The helper scripts under scripts/, such as the plotting scripts and debug.sh, are run from the repository root instead, and those pages write the paths accordingly.

sphinx-build -W is clean and ctest is 10/10.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b0aab6a6f

ℹ️ 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".

[chatgpt-codex-connector]

Preserve BeladySize's reduced hash table in MINISIM

When --algo=beladySize --profiler=MINISIM is used, this direct registry entry bypasses the BeladySize-specific reduction in bin/cachesim/cache_init.h, while MINISIM supplies hash power 20 to every profile point. BeladySize therefore allocates a 1M-slot table per miniature cache rather than the reduced power-16 table; with many profile points this can multiply memory use by 16 and trigger the algorithm's own “hash power too large” retry path or an OOM. Apply the same reduction before constructing BeladySize through the registry.

Useful? React with 👍 / 👎.

[1a1a11a]

Confirmed and fixed in b68dcee. Measured peak RSS for an 8-point MINISIM run before changing anything:

algo	before	after
beladySize	32.0 MB	10.2 MB
belady	14.8 MB	14.9 MB
lru	13.7 MB	14.7 MB

So the effect is real and specific to BeladySize, as you said.

Two corrections to the diagnosis, both in your favour on the substance:

Hyperbolic is not affected, despite having the same reduction in cache_init.h. Hyperbolic_init shrinks its own table (MAX(12, hashpower - 8) at Hyperbolic.c:47), so it converges to 12 from either entry point — which is why it measured 10.2 MB going in. BeladySize has no such line, so it really is the only one.

The retry path isn't the mechanism. BeladySize_to_evict recurses when no sampled object beats the initial score, not when the table is sparse — chained_hashtable_rand_obj_v2 loops until it hits a non-empty bucket and shrinks the table itself after 32 misses, so it never hands back NULL. The sparse table costs memory and wasted probes, not the retry. I didn't observe the warning in any run.

On where to apply it. I first did the tidier thing and moved the reduction into BeladySize_init alongside Hyperbolic's, deleting it from cache_init.h — algebraically identical for cachesim at every hash power (MAX(hp-8,16) either way; verified byte-identical miss ratios at 18, 24 and 30 against a reverted build). But testEvictionAlgo then failed:

assertion failed (miss_cnt_true[i] == res[i].n_miss): (74329 == 74347)

That test builds BeladySize at hash power 20 — the same configuration you flagged — and records the unreduced numbers. Putting the reduction in the algorithm overrides a hash power the caller explicitly asked for, so it changes results for every library caller, not just MINISIM. That is a modelling decision rather than a defect, so I left it to you and scoped this commit to the profiler that had the problem.

Worth your eye as a follow-up: BeladySize and Hyperbolic disagree about whose job this is, and the library and the CLI consequently disagree about BeladySize's effective table size. Making BeladySize self-shrink would settle it, at the cost of re-recording that test's expected counts.

MINISIM's beladySize curve does move slightly, since the sampler now draws from a right-sized table (e.g. 0.629654 → 0.626142 at wss 0.091). Confirmed deterministic across three runs, so those deltas are the fix and not sampling noise. ctest is 10/10.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b68dcee1e5

ℹ️ 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".

[chatgpt-codex-connector]

Remap BeladySize future times after spatial sampling

When MINISIM profiles beladySize with a sampling rate at or below 0.5, the reader filters the trace but leaves each oracle request's next_access_vtime in the original trace's coordinate system. BeladySize_to_evict() subtracts the sampled cache's n_req from that original timestamp when calculating its size-distance score, so the eviction priorities—and therefore the reported MRC—are invalid. Either remap future times to sampled virtual time or reject sampled BeladySize runs.

Useful? React with 👍 / 👎.

[1a1a11a]

Real, and the best finding of the review — thanks. Documented and warned in d946288.

I designed a control experiment rather than taking it on faith, since sampling always introduces error and the question is whether BeladySize has excess error. Your analysis predicts BeladySize is hit but Belady is not, because Belady uses next_access_vtime as an ordering and never takes a difference, so a constant coordinate offset cancels. MINISIM also skips sampling above rate 0.5, which gives a boundary to test across.

Error against the unsampled miss ratio, cloudPhysicsIO, 100 MB cache:

sample rate	beladySize	belady	lru
0.6 (no sampler)	0.000002	0.000014	0.000033
0.5 (sampler on)	0.0126	0.0003	0.0023
0.1	0.0245	0.0155	0.0156

At 0.5, where ordinary sampling error is still negligible, BeladySize is already 42x Belady's error and 5.5x LRU's, and it jumps discontinuously the moment the sampler switches on. Belady behaves exactly as your reasoning predicts. That is the coordinate mismatch, not sampling noise.

You offered "remap future times or reject sampled BeladySize runs". I did neither, and took a third option — warn loudly and document — for a reason worth stating.

Remapping is a change to the sampler's contract with its own accuracy trade-offs, and rejecting removes a capability over an error that is real but bounded (the curve is in the right region, just degraded). Both are modelling calls that belong to the maintainers. What was actually indefensible was reporting the number without saying it was degraded, and that is fixed: the profiler now warns when and only when the sampler is active, and doc/quickstart_mrcProfiler.md carries the mechanism and the measured numbers so anyone reading a sampled BeladySize curve knows what they have.

[WARN] beladySize scores candidates by reuse distance, which spatial sampling
distorts because next_access_vtime stays in full-trace request numbers; the
curve is approximate beyond the usual sampling error. Use
--profiler-params=FIX_RATE,1,<threads> for an exact run, or belady, which is
not affected.

Verified the warning fires under sampling and stays silent at FIX_RATE,1. ctest 10/10, sphinx-build -W clean.

---

Generated by Claude Code

[chatgpt-codex-connector]

Map FIFO-reinsertion to its actual constructor

When MINISIM or a library caller requests fifo-reinsertion through the new name registry, this entry silently constructs a Clock cache even though FIFO_Reinsertion_init is available. Experiments therefore complete successfully while measuring the wrong eviction policy; register FIFO_Reinsertion_init for this name.

Useful? React with 👍 / 👎.

[1a1a11a]

Checked this one against develop before touching it, and it isn't a regression — that mapping is pre-existing:

$ git show origin/develop:libCacheSim/bin/cachesim/cache_init.h | grep reinsertion
      {"fifo-reinsertion", Clock_init},

The registry is a verbatim move of cachesim's own table, so cachesim fifo-reinsertion has always built a Clock cache, and MINISIM now builds the same thing. Repointing the name at FIFO_Reinsertion_init would change what every existing fifo-reinsertion invocation simulates, which is exactly the kind of silent result change this PR has stayed away from — so it is yours to make, not mine to slip into a refactor whose whole claim is that it preserves behavior.

For what it's worth the alias is defensible on the merits: Clock is FIFO-with-reinsertion, and the two implementations are the same policy. So this may well be deliberate rather than a slip.

The thing genuinely worth your attention is the other half: FIFO_Reinsertion_init is reachable under no name at all, on develop or here — grep FIFO_Reinsertion against develop's table returns nothing. It is exported from evictionAlgo.h and used internally by WTinyLFU as a main-cache option, but no CLI or registry name resolves to it. So it is either dead as a standalone policy and could be documented as internal-only, or it wants its own registry entry under a distinct name. Both are one-line changes once you decide which.

---

Generated by Claude Code

[1a1a11a]

Split into five smaller PRs

This PR is now superseded — please review the five below instead. I am leaving it open until they land, since the review discussion here (15 threads, with the measurements behind each fix) is the fullest record of how these bugs were found.

PR	Contents	Base	Files
#326	Community health files, CI hardening, Apache-2.0 relicense	develop	21
#327	Documentation examples and the Read the Docs build	develop	17
#328	Crashes and memory leaks reachable from documented commands	develop	38
#329	Algorithm registry shared with the MRC profiler, --hashpower	#328	10
#330	The testCLI regression target	#329	2

#326, #327 and #328 are independent and can merge in any order. #329 and #330 stack on #328, which is why they are based on it rather than develop.

Splitting by file rather than by commit is what makes that work: every changed file belongs to exactly one PR, so nothing needs a hand-reconstructed intermediate state and no two PRs touch the same file. The cost is that #328 carries the hash-power clamps in the eviction algorithms whose motivation only appears in #329, and #327's mrcProfiler guide describes MINISIM behavior that #329 delivers.

Verification that nothing was lost or altered in the split

Merging all five branches together and diffing against this PR's head:

$ git merge claude/polish-1-repo-meta claude/polish-2-docs  # onto the 3-4-5 stack
$ git diff --quiet <union> d946288 && echo IDENTICAL
IDENTICAL

They merge without conflict and reproduce this branch byte for byte. Each was also built and tested on its own branch, not just as part of the whole: #328 and #329 are 9/9 on ctest (the tenth target is testCLI, which arrives in #330), #330 is 10/10, and #327's sphinx-build -W is clean standing alone.

Still open for you, carried over

• generalReader/binary.c normalizes a -1 next-access sentinel to match the oracle readers, but makes no attempt to reconcile any other convention a third-party binary trace might use. (Share one algorithm registry with the MRC profiler, and add --hashpower #329)
• BeladySize and Hyperbolic disagree about whose job the hash-table reduction is, so the library and the CLI disagree about BeladySize's effective table size. Settling it means re-recording testEvictionAlgo's expected counts. (Share one algorithm registry with the MRC profiler, and add --hashpower #329)
• Sampled BeladySize MRCs are approximate; remapping future access times into sampled virtual time would fix it properly. (Share one algorithm registry with the MRC profiler, and add --hashpower #329)
• fifo-reinsertion maps to Clock_init, as it always has on develop, and FIFO_Reinsertion_init is reachable under no name at all. (Share one algorithm registry with the MRC profiler, and add --hashpower #329)
• obj_id_is_num defaults to true in the library while the CLI overrides it to false, so a library caller with string ids silently collapses every id to 0. Documented, not changed. (Fix documentation examples and make the Read the Docs build work #327)

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5de5522a48

ℹ️ 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".

[chatgpt-codex-connector]

Ship the Apache license in the npm package

Declaring the addon as Apache-2.0 is not enough when the published artifact does not contain the license text. I checked npm pack --dry-run --json --ignore-scripts in libCacheSim-node; the tarball contains only six files and no LICENSE, because the repository-level license is outside the package and is not copied by the release workflow. Consequently, every npm publication distributes the statically linked library without the Apache license copy required for redistribution; copy LICENSE into this package (and include it in files) before publishing.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc77b326f0

ℹ️ 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".

[chatgpt-codex-connector]

Scale MINISIM caches by the sampler's effective rate

This only normalizes the unsampled path; now that built-in MINISIM runs execute, sampled rates whose reciprocal is not an integer still produce biased curves. For example, FIX_RATE,0.3,... is quantized by create_spatial_sampler() to sampling_ratio_inv = 3, so spatial_sample() retains 1/3 of objects and miss counts are extrapolated by 3, while line 407 sizes every cache using the unquantized 0.3 rate. The sampled workload therefore sees caches about 10% smaller than the rate represented in the results; normalize sample_rate to 1.0 / sampler->sampling_ratio_inv after creating the sampler, or reject rates that cannot be represented.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5b9472a17

ℹ️ 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".

[chatgpt-codex-connector]

Mark the oracleGeneral record as packed

When a reader uses this struct definition to generate an oracleGeneral trace, normal 64-bit C ABI padding places obj_id at offset 8 and makes the record 32 bytes, while oracleGeneralBin_setup() expects a packed 24-byte record with offsets 0, 4, 12, and 16. The resulting file is silently decoded with the wrong IDs, sizes, and future times; show __attribute__((packed)) or document the exact serialized offsets instead.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Document next access as an absolute virtual time

For anyone constructing an oracle trace, this describes the field as a reuse distance, but traceConvOracleGeneral stores the absolute request index of the next access and BeladySize explicitly computes the distance by subtracting the current request count. Encoding the documented distance therefore changes eviction priorities and yields invalid simulation results; describe it as the next access's absolute 1-based virtual timestamp, with -1 for no future access.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2a30b22a4

ℹ️ 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".

[chatgpt-codex-connector]

Stop evicting once promotion frees enough metadata

When consider_obj_metadata=true and the main policy has less metadata than the window LRU (for example, main-cache=FIFO), moving a victim from the window into the main cache can itself reduce total occupancy by 16 bytes and make enough room for the pending request. This branch nevertheless leaves evicted false, so the loop continues until it discards an additional object, causing WTinyLFU to underfill the cache and alter miss ratios. Recheck the composite capacity after the promotion and return when the pending window insertion now fits.

Useful? React with 👍 / 👎.