Check user doc cycle counts against source of truth (#3111)

[crazywriter1]

Rationale

User-facing core-lib docs copy cycle counts by hand and drift from generated MASM docs. This PR adds CI to catch that drift via explicit mappings, plus a starting assembly check with clk fixtures.

Summary

• Add scripts/check-user-doc-cycles.sh, a Python parser, and an explicit user-doc-cycle-mappings.toml to compare user-facing core-lib docs against generated MASM docs (MIDEN_BUILD_LIB_DOCS=1 cargo check -p miden-core-lib).
• Add assembly cycle fixtures with <!-- cycle-check: ... --> markers and a processor test that measures instruction cost via clk (with a baseline program to isolate the target instruction).
• Wire the check into make check-user-doc-cycles and a new CI job.
• Fix stale cycle text in user docs (collections, hashes, aead, sys) and update u32popcnt from 32 to 38 cycles based on clk measurement.

Test plan

• make check-user-doc-cycles
• cargo test -p miden-processor --lib tests::user_doc_assembly_cycle_fixtures_match_documentation -- --exact

Follow-up (out of scope for this PR)

• Expand user-doc-cycle-mappings.toml to cover remaining core-lib procedures with cycle text (e.g. find_key_value, log_precompile_request, math/u64 helpers) once generated docs expose comparable Cycles: entries.
• Add more assembly cycle-check markers and clk fixtures beyond the initial u32popcnt example.
• Longer term: generate core-lib reference tables / cycle fragments from MASM comments (issue suggestion) so CI can catch drift in inputs, outputs, and descriptions—not just cycle strings.

Closes #3111

[crazywriter1]

@huitseeker Thanks for the detailed issue. The suggested shape (explicit mapping + clk fixtures) mapped cleanly to an initial CI check.

Rationale: User docs repeat cycle counts by hand, so they drift from generated core-lib MASM docs. This adds a guardrail before that drift lands on next.

What’s in the PR:

• make check-user-doc-cycles + CI job
• user-doc-cycle-mappings.toml → compares user docs vs MIDEN_BUILD_LIB_DOCS=1 generated docs
• Assembly: cycle-check marker + clk fixture (starting with u32popcnt)

Heads-up: clk measurement (with baseline for push.7/drop) gives 38 cycles for u32popcnt, not 32 from the issue example — docs updated to match measurement.

Follow-ups (broader mappings, more assembly fixtures, generated reference blocks) are in the PR description. Happy to iterate on the assembly measurement approach if you prefer something else.

[huitseeker]

Could we avoid adding a second Markdown parser here? The core library already has a source-of-truth path: MASM doc comments generate crates/lib/core/docs/** when MIDEN_BUILD_LIB_DOCS=1 cargo check -p miden-core-lib runs.

A simpler check would be:

1. keep cycle text in the MASM doc comments,
2. regenerate the core docs in CI,
3. fail if crates/lib/core/docs/** has a diff,
4. make the user docs link to, include, or stay thin around those generated tables.

That would let us delete this Python normalizer and scripts/user-doc-cycle-mappings.toml, and it avoids maintaining a second Markdown grammar just to compare prose back to prose.

For the one assembly case, I think the same idea applies: a direct Rust assertion for u32popcnt cycle metadata, or one inline MASM test, is enough. The TOML fixture plus parser feels heavier than the check it buys us.

[crazywriter1]

@huitseeker Pushed a fix for the stack overflow: the assembly fixture test in check-user-doc-cycles.sh now uses RUST_MIN_STACK=16777216 (same as the Makefile test targets). make check-user-doc-cycles should pass end-to-end now.

On the Python parser / mapping TOML feedback: agreed that generated-doc diff + thinner user docs is the better long-term shape. Should I refactor this PR to that approach, or merge this guardrail and follow up separately? Happy to do whichever you prefer.

[crazywriter1]

@huitseeker Thanks for the follow-up review addressed both points in the latest push:

1. extra_peak_pair wording: Renamed extra_peak → extra_peak_pair in the MASM source (mmr.masm), regenerated crates/lib/core/docs/collections/mmr.md, and updated the matching user doc entry so the formula and definition use the same variable name.

2. instruction_reference.md coverage: Added <!-- cycle-check: u32popcnt --> to the u32popcnt row and a second fixture entry in assembly-cycle-fixtures.toml, so both u32_operations.md and instruction_reference.md are guarded by the cycle check.

On the earlier stack overflow feedback: the assembly fixture test in check-user-doc-cycles.sh now runs with RUST_MIN_STACK=16777216 (same as the Makefile test targets).

Still happy to pivot to the generated-doc diff approach in this PR or a follow-up let me know which you prefer.

[crazywriter1]

Addressed: restored ~ on AEAD encrypt/decrypt cycle counts, and updated extract_cycles_from_description so Cycles (estimate) / ~ are preserved in the normalized comparison (dropping the estimate marker no longer compares equal). @huitseeker

[huitseeker]

Thanks, this is much closer now.

I think there are two remaining issues to fix before merge.

The first issue is in the assembly cycle check. It still proves that the expected text appears somewhere in the marked row. It does not prove that the cycle value itself matches. A later row can pass for the wrong reason if the same number appears in the note text. The fix is to compare the cycle cell, not the whole row.

The second issue is the fixture path in the processor test. The test includes a file from ../../../scripts, which is outside the miden-processor crate. That works in this workspace, but it can fail when the crate is packaged and tested by itself. The fixture should live under a path that is packaged with the crate, and the script should read that same file.

The other comments are cleanup. I would not block on them.

[crazywriter1]

Thanks, this is much closer now.

I think there are two remaining issues to fix before merge.

The first issue is in the assembly cycle check. It still proves that the expected text appears somewhere in the marked row. It does not prove that the cycle value itself matches. A later row can pass for the wrong reason if the same number appears in the note text. The fix is to compare the cycle cell, not the whole row.

The second issue is the fixture path in the processor test. The test includes a file from ../../../scripts, which is outside the miden-processor crate. That works in this workspace, but it can fail when the crate is packaged and tested by itself. The fixture should live under a path that is packaged with the crate, and the script should read that same file.

The other comments are cleanup. I would not block on them.

Addressed all remaining review points in the latest push:

• The assembly documentation check now extracts and compares only the cycle cell, rather than scanning the full table row.
• Moved assembly-cycle-fixtures.toml under processor/src/tests/, so it is packaged with miden-processor; both the processor test and Python checker use this copy.
• Changed measured and expected cycle counts from u8 to u32.
• Removed the redundant generated_procedure field from all mappings.
• Simplified the fixture loader by removing unused optional stack_inputs / baseline handling and the unexercised begin branch.

The documentation cycle checker and processor assembly fixture test both pass. @huitseeker