Adds specifications for submodules and build targets.

[zzcgumn]

Coding agents cannot extract the purpose of a module or build target just from the code and documentation. Adding specs to the repository aims to give them this extra context. Specs should be treated as live documents, they need to be updated as requirements evolve.

[zzcgumn]

@tameware , here are the spec files that I mentioned earlier today. I have had a quick read through them and they make sense to me. As you can see I asked claude code to generate them. My suggestion would be that you check out this branch and see if cursor agrees that they are useful. My view is that next steps for what we want to do with AGENTS.md and copilot-instructions.md depends on the answer. Does this make sense to you?

[tameware]

Makes perfect sense. I'll ask Cursor to have a look now.

[tameware]

First look by Cursor. I'll add its proposed changes in my next comment. Also, this is the default Cursor model. I can apply a higher-powered one if you think it might be useful.

Copilot generated the specs added in this branch. Do they make sense?

Mostly yes — as agent-facing capability docs they have the right shape. Not yet trustworthy as source of truth without a pass for concrete wrong claims.

They follow the template well: purpose, cross-cutting invariants, entry points, non-goals. Coverage matches real subsystems (API layers, context ownership, TT, moves, concurrency, bindings, build, web). Strongest: constants-and-debug, lookup-tables, build-system, examples-cli, and much of solver-context.

Where they break down is accuracy and a few litmus slips:

Real code mismatches

• solver-context: facades do not all hold a raw ThreadData* — only MoveGenContext does; SearchContext uses shared_ptr.
• transposition-table: production lifecycle is set_memory_* → make_tt(), then init per deal — not init first. DDS_TT_CONTEXT_OWNERSHIP is wired in Bazel but unused in library sources.
• dds-public-api: “PBN twins everywhere” and “ABI = exactly dll.h + dds_c_api.h” overstate the shim and Windows export story.
• ab-stats: AB_SIDE_LOOKUP is never incremented; stats print mid-search, not on teardown.
• wasm-emscripten claims the MVP isFileURI patch; that belongs to web-mvp only.
• web-mvp misstates what web_system_tests contains; jni-ffm-binding cites a missing claude/ doc and underplays the Windows export-superset.

Template / process nits

• Every file has related-plans: [write_specs] (branch name, not a plan).
• python-binding re-lists the public surface (template says don’t).
• Some heuristics / “work-stealing” wording elevates narrative or header language into invariants.

Bottom line: sensible first draft for PR #240’s goal (give agents capability context doxygen can’t). Treat as drafts that need a fact-check pass before agents rely on them. I can land fixes for the mismatches above if you want.

[tameware]

Cursor made its proposed fixes in my local repo. Shall I submit them as updates to this PR, make a new PR, or paste the diffs here?

[zzcgumn]

I suggest that you push the updates to this branch. I'll download them and ask claude to do a code review with a high power model.

[tameware]

Pushed.

[zzcgumn]

Pushed updates from Claude's review. It did not find anything major. The references to plans are related to my local workflow.

# Review: `chore/write_specs` (range `706c742d…HEAD`)

PASS WITH NITS

Cold-context review of the 16 added files under `specs/`. Every spec was read in
full and its concrete claims sampled against the repo (BUILD files, headers,
`.bzl` files, `.lds` files, tests). **The factual accuracy is high** — I checked
~90 specific claims (targets, config settings, symbols, enum values, link flags,
test names, file paths) and all but the ones listed below hold. Notably correct
and independently verified:

- Every Bazel target named in every spec exists (`//library/src/api:dds_c_api`,
  `:api_definitions`, `//:dds`, `//:testable_dds`, `//:doxygen_docs`,
  `:solver_context{,_log,_stats}`, `:trans_table`, `:testable_trans_table`,
  `:moves`, `:testable_moves`, `:heuristic_sorting`,
  `:testable_heuristic_sorting`, `:lookup_tables`, `:system{,_util_log,_util_stats}`,
  `//library/src/utility:constants`, `//library/src:ab_stats`, `//jni:dds_shared`,
  `:dds_ffm`, `:dds_ffm_dist`, `native_in_jar_*`, `//python:{_dds3,_dds3_in_package,dds3_lib,dds3_wheel,dds3_wheel_dist}`,
  all `//wasm:*` and `//web:*` targets).
- Every config setting named exists in root `BUILD.bazel` with the stated
  `--define` and the stated `DDS_*` define in `CPPVARIABLES.bzl`
  (`debug_all`, `ab_stats`, `scheduler`, `tt_context_ownership`, `tt_reset_debug`,
  `build_{macos,linux,windows,wasm}`, `debug_build_*`).
- Every file path referenced in every spec resolves (all 60+ checked).
- `WASM_LINKOPTS` numbers (256 MB initial, 8 MB stack), `WASM_MVP_LINKOPTS`
  (`MODULARIZE=1`, `EXPORT_NAME=createDdsModule`, `_dds_mvp_calc_table`/`_malloc`/`_free`,
  `ccall`/`getValue`, `web,node`), `emsdk 5.0.7`, `DDS_FFM_VERSION = "3.0.1"`,
  Python `3.10+` (docs/python_interface.md:10), wheel `version = "1.0.0"` and
  `strip_path_prefixes = ["python/"]` — all exact.
- `jni/version_script.lds` contains exactly the `dll.h` flat set plus the five
  `dds_c_*` symbols, and `dds_api.hpp` marks the modern `dds_*` symbols
  `EXTERN_C DLLEXPORT` with no `.lds` branch on Windows — so
  `dds-public-api.md:62-67` and `jni-ffm-binding.md:35-44`'s "Windows exports a
  superset" claim is correct and non-obvious.
- The `SolverContext` ownership model is described accurately down to details I
  expected to be wrong: `~SolverContext()` really does `thr_.use_count() == 1` →
  `close_debug_files()` (solver_context.cpp:152-162); `MoveGenContext` really
  holds a raw `ThreadData*` (solver_context.hpp:419) while `SearchContext` holds
  a `shared_ptr` + `unique_ptr<TransTable>` (:304, :306); the env overrides
  `DDS_TT_DEFAULT_MB`/`DDS_TT_LIMIT_MB` and the `THREADMEM_*` fallback are
  exactly as described (solver_context.cpp:66-92).
- The two "the header is wrong, here is what the code does" corrections are both
  real: `parallel_boards.hpp:26` says "work-stealing" while the implementation is
  a shared `next.fetch_add` (parallel_boards.cpp:93), and
  `solver_context_adapter.cpp:12` really does still say "Use ThreadData-attached
  TT". Flagging those in the specs is exactly the right call.
- `AB_SIDE_LOOKUP` really is a named-but-never-incremented enum slot
  (only hits: `ab_stats.hpp:38`, `ab_stats.cpp:89`), `MgType::SIZE == 13`,
  `NodeCards` is 8 bytes, `ResetReason` values match, `bit_map_rank[2] == 0x0001`
  / `[14] == 0x1000` with sentinels at 0/1/15, all six lookup tables and their
  `[8192]`/`[8192][15]`/`[8192][14]` shapes match `lookup_tables.hpp`.

**Plan's structural validation commands: both pass.**
`ls specs/*.md | grep -v _template | wc -l` → `15`. The front-matter/section
grep loop prints nothing (no `INCOMPLETE:` lines). All 16 files have a
well-formed `---` front-matter block.

## Blocking

None.

## Should-fix

### 1. `specs/solver-context.md:52-54` — the `clear_tt()` reuse claim is false

The spec states:

> `clear_tt()` calls `return_all_memory()` on the existing TT object (it does not
> destroy the `unique_ptr`; **the next deal's `init` / use reallocates inside the
> same table**)

The first half is right (`solver_context.cpp:198-205` calls
`tt->return_all_memory()` and leaves `tt_` alive). The bolded half is wrong:

- `library/src/trans_table/trans_table.hpp:151-154` documents
  `return_all_memory()` as "Deallocates all structures. **The table must be
  re-initialized with `make_tt()` before further use.**"
- `TransTable::init(hand_lookup)` does **not** allocate anything — both
  `TransTableL::init` (trans_table_l.cpp:234) and `TransTableS::init`
  (trans_table_s.cpp:91) only fill the `aggr_`/`aggp_` aggregate lookup arrays.
  Allocation lives in `make_tt()`/`init_tt()` (trans_table_l.cpp:355,
  trans_table_s.cpp:245-260).
- `SolverContext::SearchContext::trans_table()` (solver_context.cpp:66) returns
  early on non-null `tt_`, so `make_tt()` is never re-run on the context path.
  `init.cpp:259` then calls `ctx.trans_table()->init(handLookup)` on the
  deallocated table.
- The actual post-`clear_tt()` behaviour is a silently dead cache, not
  reallocation: `TransTableL::add` returns immediately when
  `last_block_seen_[tricks][hand] == nullptr` "we have recently reset the entire
  memory … drop this entry" (trans_table_l.cpp:956-961), and `TransTableS`
  lookup/return paths short-circuit on `!tt_in_use_` (trans_table_s.cpp:150, 374).

Nothing enforces the stated invariant. `context_tt_facade_test.cpp:29` is the
only test that touches `clear_tt()`, and it calls it in the *no-TT-exists*
no-op case — it never clears a live table and then reuses it.

Fix: replace the parenthetical with what the code does, e.g. "…it does not
destroy the `unique_ptr`, but the table's storage is gone and the context never
re-runs `make_tt()`, so a context reused after `clear_tt()` degrades to a
no-op cache. Use `dispose_trans_table()` if you want a fresh table." Also
reconcile with `specs/transposition-table.md:50-51`, which is closer
("requires `make_tt()` / in-table reallocation before reuse") but is vague enough
to have licensed the wrong statement here.

### 2. All 15 specs — `related-plans: []` contradicts the plan and the template

Every spec has `related-plans: []` (ab-stats.md:5, build-system.md:5,
constants-and-debug.md:5, dds-public-api.md:5, examples-cli.md:5,
heuristic-sorting.md:5, jni-ffm-binding.md:5, lookup-tables.md:5,
move-generation.md:5, python-binding.md:5, solver-context.md:5,
system-concurrency.md:5, transposition-table.md:5, wasm-emscripten.md:5,
web-mvp.md:5).

The plan's "What changes" section specifies the front-matter block verbatim as
`related-plans: [write_specs]`, and `specs/_template.md:5` requires
`related-plans: [<plan-name>, ...]`. Every spec in this change was authored by
the `write_specs` plan, so the field should be populated. As-is the field is
dead weight and the provenance trail the template exists to create is empty on
day one.

Fix: `related-plans: [write_specs]` in all 15.

### 3. Cross-links use `[[wiki-link]]` syntax, which does not render as a link

All 15 specs cross-reference each other as `[[build-system]]`,
`[[solver-context]]`, `[[transposition-table]]`, etc. (e.g. ab-stats.md:33-34,
dds-public-api.md:41, transposition-table.md:20, system-concurrency.md:22).
This is Obsidian/wiki syntax; GitHub-Flavored Markdown renders it as the literal
text `[[build-system]]` with no link.

The plan's stated motivation is "Collaborators (**not all of whom use Claude
Code**) need durable … specifications", and the plan file itself uses standard
markdown links (`[claude/analysis/write_specs.md](../analysis/write_specs.md)`).
So the repo convention is markdown links, and the audience is precisely the one
that will see broken text. Nothing in `specs/_template.md` mandates `[[...]]`.

Fix: `[build-system](build-system.md)` etc. (all specs are siblings in `specs/`,
so the relative path is just the filename).

## Nits

1. **`specs/ab-stats.md:58`** — "Call sites: `library/src/ab_search.cpp`,
   `solver_if.cpp` (via `AB_COUNT`)". `AB_COUNT` appears in exactly two files:
   `ab_stats.hpp` (definition) and `ab_search.cpp`. `solver_if.cpp` touches
   `ABStats` directly (`thrp->ABStats.Reset()/.ResetCum()` at :280-281, :717-718,
   :889-890) but never through `AB_COUNT`. Fix: "`ab_search.cpp` (via `AB_COUNT`);
   `solver_if.cpp` (direct `Reset`/`ResetCum`)".

2. **`specs/dds-public-api.md:58-61`** — "**many** `dds_api.hpp` entry points have
   a `*PBN` twin". `dds_api.hpp` has exactly one: `dds_calc_dd_table_pbn` (:55).
   `dds_solve_board` (:42) and `dds_calc_par` (:60) have none. The `dll.h` half of
   the sentence is fine. Fix: "Most flat `dll.h` entry points have a `*PBN` twin;
   on the modern C++ layer only `dds_calc_dd_table` does."

3. **`specs/dds-public-api.md:34-36`** — flat layer described as "Declared
   `EXTERN_C DLLEXPORT auto STDCALL … -> int`", but the same bullet lists
   `SetMaxThreads`, `SetResources`, `FreeMemory`, which are `-> void`
   (`dll.h:439`, `:503`). Fix: drop `-> int` from the signature sketch (the
   "Integer status returns" bullet at :56-57 already scopes it correctly to
   solver entry points).

4. **Bare package labels used as target labels** —
   `system-concurrency.md:75` (`//library/tests/system`),
   `move-generation.md:65` (`//library/tests/moves`),
   `transposition-table.md:76` (`//library/tests/trans_table`),
   `heuristic-sorting.md:63-64` (`//library/tests/heuristic_sorting`).
   Of these, only `//library/tests/trans_table` and
   `//library/tests/regression/heuristic_sorting` happen to have a same-named
   target. `bazel test //library/tests/moves` and
   `bazel test //library/tests/heuristic_sorting` both fail ("no such target") —
   the targets are `moves_test` and `heuristic_sorting_test`/`merge_scratch_test`/
   `minimal_new_test`/`minimal_weight_test`/`targeted_unit_tests`. The plan's own
   wording ("guarded by `library/tests/system`") licensed this, but a reader will
   copy-paste it. Fix: use `//library/tests/moves/...` form, or name the targets.

5. **`specs/build-system.md:31-32`** — "`build_{macos,linux,windows}` match the
   OS". `build_windows` and `debug_build_windows` also require
   `@platforms//cpu:x86_64` (`BUILD.bazel:33-40`, `:43-50`), so a Windows-arm64
   build matches neither and falls into `//conditions:default`. Worth one clause,
   since it is exactly the kind of matrix hole a future porter would trust the
   spec on.

6. **`specs/lookup-tables.md:47-49`** — "per-group `rank_`, `sequence_`,
   `fullseq_`, and `gap_` arrays, valid only for indices `0..last_group_`".
   `lookup_tables.hpp:72` documents `gap_` as "Only indices **1**..last_group_
   contain valid gap data" (a gap is between runs). Fix: carve out `gap_`.

7. **`specs/transposition-table.md:62-64` / `specs/build-system.md:49`** —
   `tt_context_ownership` is described as "unused in library sources today",
   which is true of `library/src/`, but the define is actually applied to a test
   target: `library/tests/system/BUILD.bazel:42`
   (`context_equivalence_test_ctx_tt`, `local_defines = ["DDS_TT_CONTEXT_OWNERSHIP"]`).
   No source anywhere `#ifdef`s it, so the define is inert on that target too.
   Worth saying so explicitly, otherwise the next reader who greps finds the test
   and concludes the spec is stale.

8. **`specs/examples-cli.md:45-47`** — "`examples/BUILD.bazel` **may** alias it".
   It unconditionally does: `examples/BUILD.bazel:281-285` is an `alias(name =
   "all_examples_wasm", actual = "//wasm:all_examples_wasm")` with the comment
   "Backward-compatible alias; prefer //wasm:all_examples_wasm." State the fact.

9. **`specs/_template.md` landed inside this range, so the diff is 16 files, not
   the plan's 15.** The plan lists `specs/_template.md` as an "Unchanged
   reference file … not edited", but `git log` shows it was *added* by
   `061c4222 adds a template for writing new specs.` — and added **last**, after
   all 15 specs (`19b05a41` … `e8aa26e4`). So the template did not exist on this
   branch while the specs that claim to follow it were being written; it was
   back-filled. Substantively this is fine — the specs do conform to the template
   as committed, and the plan's "expect 15" check passes because it filters
   `_template` — but the plan's premise ("there is a `specs/_template.md`") was
   false at the time and the ordering means the template was reverse-engineered
   from the specs rather than the other way round. No action needed beyond
   awareness; the plan's "What does NOT change: `specs/_template.md` is not
   modified" is technically honoured.

10. **Two plan anchors were dropped without comment.** The plan's
    capability-specific anchor list asks jni-ffm-binding to reference
    `claude/analysis/jni_shared_library.md` — no spec references it (the file
    exists). And it asks solver-context to capture the "`SolverContext`
    (SPM-safe public header) vs. `SolverContextImpl` (inline private) split" —
    `SolverContextImpl` does not exist anywhere in the tree, so omitting it is
    *correct* (the self-correction commit presumably caught this). The first is a
    genuine miss; the second is a good call the plan got wrong. Fix: add the
    `claude/analysis/jni_shared_library.md` pointer to
    `specs/jni-ffm-binding.md`'s "Key entry points".

11. **`last-updated: 2026-07-18` vs the plan's specimen `2026-07-16`.** All 15
    use `2026-07-18`, which is today and matches the correction commit. Correct
    behaviour; noting only because the plan text specifies a different literal.

## Internal consistency

I looked specifically for contradictions between specs on shared mechanisms and
found none beyond finding #1's `clear_tt()` looseness:

- Single-thread-per-context is stated consistently in `dds-public-api.md:52-55`,
  `solver-context.md:30-31`, `transposition-table.md:34-36`,
  `move-generation.md:73-74`, `jni-ffm-binding.md:96-97`.
- "Link at most one log/stats variant into a binary" agrees between
  `solver-context.md:66` and `system-concurrency.md:57`.
- The WASM single-thread assumption agrees between `system-concurrency.md:80-81`,
  `wasm-emscripten.md:37-38`, `web-mvp.md:57-58`.
- `TTKind` default `Large` agrees between `solver-context.md:44` and
  `transposition-table.md:32-33` (they list the enumerators in different orders,
  `{Small,Large}` vs `{Large,Small}`; the declaration order is
  `Small, Large` — cosmetic only).
- The `ab_stats` → `DDS_AB_STATS` → `DDS_LOCAL_DEFINES` chain agrees between
  `ab-stats.md:31-34`, `constants-and-debug.md:51-54` and `build-system.md:47`.
- `WASM_LINKOPTS` ownership: `build-system.md:55-58` owns it, `wasm-emscripten.md:38-41`
  and `web-mvp.md:33-34` defer to it. No duplication drift.

No TODOs, no unfilled `<placeholder>` fields, and no leftover template prose in
any of the 15 specs.

[zzcgumn]

Looks like we will have only a couple of nits if we do one more back and forth between Cursor and Claude.

[tameware]

Pushed changes courtesy of Grok 4.5 High Fast:

Mostly solid after the fact-check commits. Shape and coverage are right for agent context; a few remaining factual errors still matter.

Still wrong / risky

1. dds-public-api.md (high) — Says Java, .NET, and ctypes bind to dds_c_*. .NET actually P/Invokes modern dds_* from dds_api.hpp (DdsNative.cs). No ctypes binding ships. Java/FFM → dds_c_* is correct.

2. solver-context.md (high) — The “clear_tt() → silently dead cache” story is partly wrong. Keeping unique_ptr and not re-running make_tt() is right; claiming TransTableS short-circuits on !tt_in_use_ is not (tt_in_use_ is only checked in make_tt / return_all_memory). Reuse after clear_tt() is closer to UAF/crash than a quiet miss. Also: DDS_TT_DEFAULT_MB overrides defMB, it does not “raise” it.

3. transposition-table.md (medium) — Soft/hard two-tier limits overstate TransTableS: set_memory_default is a no-op there.

4. heuristic-sorting.md (medium) — Contradicts itself: body says code wins when doc/heuristic-sorting.md lags; non-goals still call that doc “source of truth.”

Minor

• system-concurrency.md — Could note malformed order falls back to index order (already implemented + tested).
• build-system.md — owners mix of // and filenames is awkward vs peers.
• dds-public-api.md — Still a bit of dll.h symbol catalog (litmus).

Clean enough

ab-stats, constants-and-debug, lookup-tables, move-generation, examples-cli, python-binding, jni-ffm-binding, wasm-emscripten, web-mvp.

[zzcgumn]

Pushed one medium and four nits identified by claude using opus 4.8.

[tameware]

LGTM