Add fft/spectrum.h: the packed-spectrum view; migrate pvoc, log_mel, test_pvoc (Stage 5)

[tap]

What this changes

Stage 5 of the FFT plan (docs/audit-fft-and-code-smells.md, Part 1 F8 / Part 3 Stage 5 / Part 13). Adds include/tap/dsp/fft/spectrum.h, a non-owning packed_spectrum<Sample> view over a DspTap packed real spectrum, and migrates the four hand-indexed sites in pvoc.h, the power-spectrum site in log_mel.h, and test_pvoc.cpp's band_energy onto it. tests/test_spectrum.cpp pins the view's definition against real forward and inverse transforms. tools/fingerprint/ commits the same-host A/B fingerprint tool this PR's gate was measured with.

The view's API (all [[nodiscard]] constexpr noexcept, no allocation, no virtuals; Sample may be const-qualified for a read-only view, a mutable view converts implicitly to the const one, and CTAD follows the pointer's constness through the implicit guide). Value types: float, double (the floating profiles) and int16_t, int32_t (the Q15/Q31 profiles, which present the same packing), so Stage 3 does not reopen the header:

accessor	meaning
packed_spectrum(Sample* data, std::size_t n)	view N packed values; non-owning (pointer + size), valid only while the buffer is; @pre N even, >= 2 (deliberately looser than fft.h's power-of-two >= 4)
size() / num_bins() / data()	N, N/2 + 1, the buffer
dc() / nyquist()	a[0] / a[1], by reference
re(k) / im(k), 1 <= k < N/2	a[2k] / a[2k+1], by reference, native exp(+i) convention
power(k), 0 <= k <= N/2	re*re + im*im in power_type, DC and Nyquist included; no one-sided factor 2, no 1/N; Parseval over the packing is in the docstring
power_type	the sample type for float/double (the hand-written consumers' exact expression); int64_t for int16/int32, promoted before the multiply (exact except re == im == INT32_MIN, documented)
bin_engineering(k), floating profiles only	std::complex<Sample>(a[2k], -a[2k+1]): the one convention-flipping accessor; its name and docstring say it conjugates

The docstring carries the numeric definition (bin[k] = a[2k] + i·a[2k+1], DC at a[0], Nyquist at a[1], W = exp(+2πi/N), inverse unnormalized, 2/N for a round trip). fft.h is untouched in this wave (see the follow-ups below).

Why

The packing was re-derived by hand at every consumer (F8), and the plan wants one shared, tested definition in place before the FFT itself changes, so the ported engine and the fixed-point profiles have a single seam to re-present. Per the adversarial review (P2), the native accessors are primary and the migration is DspTap-only; MuTap adopts the view per header when each is next touched, tracked in tap/MuTap#51.

Verification

Built and ran on this Linux host, at 5631d35 (= c609c5e plus a one-line MSVC C4310 fix in the promotion test, which the Windows leg caught):

cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DTAP_DSP_WERROR=ON     # gcc 13.3
cmake --build build -j8                                                  # zero warnings
ctest --test-dir build                                                   # 191/191 passed (31 packed_spectrum tests)
# same configure with -DCMAKE_CXX_COMPILER=clang++ (clang 18) in a scratch dir: zero warnings, 191/191
scripts/tidy.sh tests/test_spectrum.cpp tests/test_pvoc.cpp tests/test_log_mel.cpp   # clang-tidy clean (a subset; CI scans every project TU)
clang-format --dry-run --Werror                                          # clean on every touched file
cmake -B build_fp -S tools/fingerprint -DCMAKE_BUILD_TYPE=Release && cmake --build build_fp   # the tool builds standalone, -Werror clean

Bit identity (the gate). tools/fingerprint/dsptap_fingerprint runs basic_pvoc at ratio 1.0 and 1.5, formant off and on, float and double (N=1024, 48000 samples) and basic_log_mel on the log and PCEN paths, float and double (default geometry, 300 frames), over a fixed xorshift32-plus-two-tones corpus, and prints the FNV-1a-64 fingerprint of each case's raw output bytes. Same tool source, compiled against the tree before this PR and at the PR head, gcc 13.3 -O3 -DNDEBUG on this host; also cross-checked with clang at -O3 and gcc at -O0 with asserts on. Every line is identical:

case	before	after
pvoc float ratio 1.0 formant off	a72e59ec1ae77c1b	a72e59ec1ae77c1b
pvoc float ratio 1.5 formant off	361fd93084aa3e3d	361fd93084aa3e3d
pvoc float ratio 1.0 formant on	a72e59ec1ae77c1b	a72e59ec1ae77c1b
pvoc float ratio 1.5 formant on	a094126eff3a28a6	a094126eff3a28a6
pvoc double ratio 1.0 formant off	b05b35b0c3316234	b05b35b0c3316234
pvoc double ratio 1.5 formant off	2b01c6b624b9346f	2b01c6b624b9346f
pvoc double ratio 1.0 formant on	b05b35b0c3316234	b05b35b0c3316234
pvoc double ratio 1.5 formant on	01efab7e17e7c8e0	01efab7e17e7c8e0
log_mel float log	ca12a268877e9569	ca12a268877e9569
log_mel float PCEN	b65095f086706f6c	b65095f086706f6c
log_mel double log	bcec290b78fbf456	bcec290b78fbf456
log_mel double PCEN	98d680254e6363cc	98d680254e6363cc

These hashes are specific to this host, compiler, flags and libm (pvoc runs atan2/sin/cos in double); they are evidence of an A/B on one host, not golden values, and the tool's header says so. (Ratio 1.0 with formant on and off fingerprinting identically is the documented identity: the correction is exactly unity when nothing moves.) Floating-point expression order is unchanged at every site: re*re + im*im stays in that order and in the same type, the LPC magnitude and the analysis magnitude still square in double after the cast, and the conjugation in bin_engineering() is a sign flip in Sample before the cast to double, which is exact. Both hostile reviewers reproduced the identity independently with their own harnesses, including against the #19 + #20 merge.

Not run here: the macOS vDSP and Windows legs and the M55 compile leg, which CI covers. The notebooks were not re-executed because no primitive's behavior changed (the fingerprints are the evidence).

Notes for the reviewer

• No contract change. The packing, conventions and normalization are restated in the view's docstring, not moved.
• Mid-wave file ownership: fft/spectrum.h (new), pvoc.h, log_mel.h, test_pvoc.cpp, test_spectrum.cpp (new), tools/fingerprint/ (new, own CMake project like tools/capi; root CMake untouched), and tests/CMakeLists.txt, where the test_yin.cpp) line is split to insert test_spectrum.cpp alphabetically before it. pvoc.h's sample counter (m_n, the process() ring indexing) is left alone for Stage 0 (Bound the psola/pvoc sample clocks so 32-bit targets never overflow (Stage 0) #20); the only pvoc edits are in run_frame's spectrum indexing, the synthesis pack and compute_envelope's magnitude loop. git merge-tree against Bound the psola/pvoc sample clocks so 32-bit targets never overflow (Stage 0) #20 is a clean auto-merge (both reviewers confirmed).
• Merge order: per the Part 12 table, after Stage 3a: sample_traits<double>, named Q ladder, fft_arith trait, decimate re-pin #22 (Stage 3a), which also touches tests/CMakeLists.txt (appends test_fft_arith.cpp) and also creates include/tap/dsp/fft/. The resolution is keeping both source lines.
• log_mel.h's power loop still peels DC and Nyquist and iterates 1..num_bins()-1 with power(k), so the hot loop stays branch-light after inlining (reviewer A confirmed VRP hoists the edge branches).
• The [[nodiscard]] attribute is new to include/ (the brief asked for it); STYLE.md is silent on it. Flagged for the taphouse maintainers to decide and record; removing it is mechanical if the answer is no.
• Follow-ups outside this PR's ownership, recorded here so they are not lost:
  • fft.h's class docstring still carries the packing table; in the wave-2 pass replace the three packing bullets with one sentence pointing at fft/spectrum.h, keeping the sign-convention and normalization sentences.
  • README: no section for the view (a view is not a primitive; the primitive count and the checklist do not move), but the FFT "Key contract points" restate the packing and should gain one line: "The packing is defined once, with tested accessors, in include/tap/dsp/fft/spectrum.h." Owned by the README owners in this wave; take it in the same wave-2 pass as fft.h.
  • Run the test battery bare-metal under QEMU on Cortex-M4, M33 and M55 #17's bare_metal_main.cpp positive filter does not include packed_spectrum_test/*, packed_spectrum_slot_test/*, packed_spectrum_contract.*; Part 13 makes the filter negative, which resolves this, and the request is on record for whichever of Run the test battery bare-metal under QEMU on Cortex-M4, M33 and M55 #17/Add fft/spectrum.h: the packed-spectrum view; migrate pvoc, log_mel, test_pvoc (Stage 5) #19 merges second.
  • MuTap adoption of the view per header (77 sites; native accessors only in the hot loops, never bin_engineering(); gated by fingerprint identity and 0% icount): Adopt tap::dsp::packed_spectrum per header (77 sites) MuTap#51.

🤖 Generated with Claude Code

https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy

[tap]

Hostile review B (process / downstream)

Scope: ownership, truthfulness of the PR body, plan compliance (Stage 5 / F8 / Part 12), cross-PR merge hazards inside wave 1, downstream consumers, style/process. Numerics were reviewer A's brief; I only touched them where the process claims depend on them. Everything below was checked against the branch at f76aa40, origin/main at 5ca3b1c, the other wave-1 branches, the CI logs, and a scratch three-way merge with #20.

What I verified (evidence for the verdict, not praise)

• Ownership. Files touched: include/tap/dsp/fft/spectrum.h (new), include/tap/dsp/log_mel.h, include/tap/dsp/pvoc.h, tests/CMakeLists.txt (one list line), tests/test_pvoc.cpp, tests/test_spectrum.cpp (new). Exactly the Part 12 allotment; fft.h, README, ci.yml, root CMake untouched.
• Stage 0 lines in pvoc.h. git diff -U0 hunk starts: this PR at 28, 36, 41, 184, 187, 221, 255, 265, 330, 332, 334; #20 at 37, 50, 132, 144, 149, 156, 161, 164, 167, 274, 365. No hunk overlaps; the nearest pair (this PR's 265–266 vs #20's 274–275) has seven untouched lines between them. git merge-tree --write-tree of the two heads: clean auto-merge of pvoc.h and test_pvoc.cpp.
• Build/tests, this branch. cmake -DCMAKE_BUILD_TYPE=Release -DTAP_DSP_WERROR=ON, gcc 13.3: 0 warnings, ctest 175/175 (15 packed_spectrum tests, as the body says). CI: two runs (push 35257433698/35257434564, PR 35257508540/35257510114), all 12 check runs green; the clang-tidy job log ends clang-tidy clean. and its file list is built from compile_commands.json, so test_spectrum.cpp and, through it, fft/spectrum.h were scanned.
• Bit identity, independently reproduced. I wrote my own FNV-1a-64 harness (different signal from yours, so the hashes differ from the table in the body; that is the point) over basic_pvoc (N=1024, 48000 samples, ratio 1.0/1.5 × formant off/on × float/double) and basic_log_mel (default geometry, 300 frames, log/PCEN × float/double), built against origin/main, this branch, #20 alone, and the #19+#20 merge, with gcc and clang at -O3 -DNDEBUG and at -O0 (asserts on). All 12 cases × 4 configurations are byte-identical across all four trees (e689e7f8… / 713095c0… for the ratio-1.0 rows, etc.). The ratio-1.0 formant-on/off pairs coincide, as your body predicts.
• Merged tree (#19 + #20). Clean merge, -DTAP_DSP_WERROR=ON 0 warnings, ctest 179/179 (175 + #20's four overflow tests), pvoc/psola/spectrum subset 47/47. The merged pvoc output is bit-identical to origin/main per the harness above. So the merge is proven, not just each side.
• Downstream. MuTap and MuTap-Max C++ sources include neither pvoc.h nor log_mel.h (grep, excluding the submodule). MuTap's tools/ml/kws/kws_features.py does consume log_mel through the DspTap C ABI bridge and keys its dataset lock to the DspTap commit; log_mel identity (proven above, both profiles) means a future pin bump does not invalidate the toy-fixture lock. Both consumers currently pin 5ca3b1c = today's origin/main, so there is no bump urgency from this PR. The body says the notebook was not re-executed because behavior is unchanged; correct, pitchshift.ipynb measures pvoc and identity makes re-execution moot.
• Process. Commit carries Co-Authored-By and Claude-Session trailers; PR body follows the template (all four sections, the "no contract change" note present, the footer present).

Findings

1. should-fix (plan compliance): "Ooura contract" wording is still in a consumer header.
Plan Stage 5 / P13 ("the view's docstring is where the numeric definition lives once 'Ooura contract' leaves the consumer headers"). This PR removes the // Ooura packing: comment at log_mel.h:310 but leaves log_mel.h:35: unnormalized real DFT of tap::dsp::basic_real_fft (Ooura contract), so. That line is in a file this PR owns and is the same wording the stage exists to retire; after Stage 2c the sentence becomes false.
Fix: … of tap::dsp::basic_real_fft (the packed spectrum defined in fft/spectrum.h), so …. Comment-only, no fingerprint impact.

2. should-fix (cross-PR): tests/CMakeLists.txt conflicts with #22 (Stage 3a); merge order and a trivial resolution.
git merge-tree of this head against every other wave-1 branch: clean with #17, #20, #21, #23, #24, docs-provenance; CONFLICT (content) in tests/CMakeLists.txt with #22, because both PRs replace the same test_yin.cpp) line to append one file (test_fft_arith.cpp vs test_spectrum.cpp). The PR body's "one source line at the end of tests/CMakeLists.txt" undersells it: appending after the closing paren is exactly the pattern that makes two one-liners collide. Both also create include/tap/dsp/fft/ (fft_arith.h vs spectrum.h); different files, no conflict, just noting the directory is now shared.
Fix: follow the Part 12 table order (#20 → #17 → #21 → #24 → #22 → #19 → #18 → #23); when #19 rebases onto #22 the resolution is keeping both lines. Nit inside the fix: the source list is alphabetical and test_spectrum.cpp belongs between test_sample_traits.cpp and test_yin.cpp, not after it. #17 and #24 also edit this file (a restructuring and a large trailing block, respectively) but auto-merge cleanly with this PR; I did not build those combinations.

3. should-fix (follow-up, not this PR's files): the new suite is invisible to the QEMU legs once #17 lands.
#17's tests/bare_metal_main.cpp is a positive filter (real_fft_test/0.*:…:pvoc_test/0.*:pvoc_cross_precision.*); packed_spectrum_test/* and packed_spectrum_contract.* are not in it, so after both merge the view's contract runs on the three hosts only. The view is tiny and float-only-safe, so there is no size argument for excluding it.
Fix: request to the #17 owner (or the rebase of whichever merges second): add packed_spectrum_test/0.*:packed_spectrum_contract.* to the filter. Recording it here so it is not lost between owners.

4. should-fix (truthfulness / reproducibility): the fingerprint evidence is not reproducible from the repository.
The 12-row table in the body is the gate for this PR (plan table row 5: "pinned pvoc/log_mel tests"; F8's whole point is a bit-identical migration), and the harness that produced it is "scratchpad only, not in the repo". A reviewer cannot re-run it, a rebase onto #20 cannot re-run it, and Stage 6's DspTap gate ("fingerprint on Hann/pi change") presupposes a DspTap-side fingerprint tool that does not exist; the plan's Stage 1 harness lives in MuTap and covers MuTap's chain, not pvoc/log_mel. I had to rebuild one from scratch to prove the #19+#20 merge (it took a few dozen lines; see above).
Recommendation: do not commit golden hashes as a gtest pass/fail: pvoc goes through atan2/sin/cos in double and the hashes will differ between the Linux, macOS and Windows libm, so a pinned hash would fail on two of the three CI hosts by design. Do commit the harness itself as a same-host A/B tool that prints the table: a standalone tools/fingerprint/ (own CMakeLists.txt, like tools/capi, so root CMake stays untouched) with the xorshift32-plus-two-tones signal, the 12 cases, and a README line saying "run against two trees on one host, diff the output". That makes every future pvoc/log_mel touch (Stage 2b routing flip, Stage 4 engine parameter, Stage 6 Hann/pi) gate-able the way this PR claims to be, and makes this PR's evidence reproducible. It can ride in this PR (new directory, no owner conflict) or immediately after.

5. should-fix (plan F8 / MuTap deferral): the deferral is stated but not tracked.
The body says "MuTap adopts the view per header when each is next touched", which matches Stage 5's text, but nothing records the 77 sites (fdaf.h 25, fd_kalman.h 28, nn_suppressor.h 15, postfilter.h 9), the per-header gate (fingerprint identical + 0% icount), or the fact that MuTap's hot loops do native-convention complex products by hand and must not use bin_engineering(). The next agent to touch fdaf.h will not read this PR.
Fix: open one MuTap tracking issue titled for the view, listing the four headers with site counts, the gate, and the "native accessors only in the ratcheted loops" rule; link it from the PR body under "Notes for the reviewer". No DspTap file changes.

6. nit (follow-ups outside this PR's ownership): fft.h and README pointers.

• fft.h's class docstring (lines 318–331 on main) still carries the packing table itself. The plan makes spectrum.h the one home; the PR body already says "fft.h is untouched in this wave; its docstring can point at the view later". Nobody owns fft.h in wave 1, so this is a wave-2 request: replace the three packing bullets with one sentence pointing at fft/spectrum.h, keeping the sign-convention and normalization sentences where they are.
• README: no section for the view. Decision: a view is not a primitive (the "seven primitives" count and the Adding-a-primitive checklist should not move), so a full section is wrong; but the README's "Key contract points" for the FFT restates the packing (data[0] = DC …) and should gain one line: "The packing is defined once, with tested accessors, in include/tap/dsp/fft/spectrum.h." README is owned by four other wave-1 PRs (#17, #21, #22, #23); request it from whichever README owner merges last, or take it in the same wave-2 pass as fft.h.

7. nit (house style): [[nodiscard]] enters include/ for the first time; STYLE.md is silent.
grep -rn nodiscard include/ on main is empty; STYLE.md has no rule either way (no hit for nodiscard or attribute). The PR body discloses this, which is the right move. Not blocking: the attribute is correct on every accessor here (note that the reference-returning dc()/re(k)/im(k) are used as lvalues in pvoc's synthesis pack, which does not trigger the diagnostic). Flag for the taphouse maintainers: decide whether [[nodiscard]] is house style for pure accessors, and record it in STYLE.md so the next primitive does not have to guess. If the answer is "no", removing it from the view is mechanical.

8. nit (PR body precision). "one source line at the end of tests/CMakeLists.txt" is really "the test_yin.cpp) line is split to append one entry" (see finding 2). And the body's tidy command lists tests/test_log_mel.cpp (exists, correct) but the CI gate scans all project TUs, so the local three-file run was a subset; fine, but say "subset" when the body cites it as the tidy evidence.

Verdict

Not a blocker in the set: merge after #22 with the one-line tests/CMakeLists.txt resolution, fix the log_mel.h:35 wording (finding 1) before or during that rebase, and land the fingerprint tool (finding 4) either here or in the next PR so this PR's headline evidence stops living only in a scratch directory.

---

Generated by Claude Code

[tap]

Hostile review A (correctness)

Reviewed f76aa40 against origin/main (5ca3b1c). Built in a private scratch dir with -DTAP_DSP_WERROR=ON under gcc 13.3 and clang 18.1 (175/175 tests pass on both); clang-format --dry-run --Werror and scripts/tidy.sh clean on every touched file. No arm-none-eabi toolchain on this host, so the M55 compile leg is CI's.

Findings

1. should-fix (do it in this PR, the header is new): the static_assert forecloses the Stage 3 fixed-point views, and a naive relaxation would be wrong.
spectrum.h:55 accepts only float/double. Part 7 says the Q15/Q31 profiles present the same packed spectrum, and Part 9 types test_fft.cpp over float, double, int16_t, int32_t; Stage 3 will therefore have to reopen this header. Evidence that just widening the assert is not enough: with is_arithmetic_v substituted, packed_spectrum<const int16_t> over a = {0,0,30000,30000,...} returns power(1) == -11776 (true |X|^2 = 1 800 000 000) because value_type is int16_t and the int product is narrowed back (gcc -Wconversion warns at line 111); and bin_engineering() would instantiate std::complex<int16_t>, which [complex.numbers]/2 leaves unspecified.
Minimal change that keeps float/double bit-identical (same type, same operand order) and does not need revisiting in 3b:

static_assert(std::is_same_v<value_type, float> || std::is_same_v<value_type, double>
              || std::is_same_v<value_type, std::int16_t> || std::is_same_v<value_type, std::int32_t>, "...");
/// Type of power(): the sample type for the floating profiles, int64 for the fixed-point ones
/// (exact for any int32 pair except re == im == INT32_MIN, which Part 7's scaling excludes; say so).
using power_type = std::conditional_t<std::is_integral_v<value_type>, std::int64_t, value_type>;
[[nodiscard]] constexpr power_type power(std::size_t k) const noexcept {
    ...
    const power_type re = m_data[2 * k];   // promotion before the multiply
    const power_type im = m_data[2 * k + 1];
    return re * re + im * im;
}
[[nodiscard]] constexpr std::complex<value_type> bin_engineering(std::size_t k) const noexcept
    requires std::is_floating_point_v<value_type>;   // std::complex<int> is unspecified

plus int16_t/int32_t added to sample_types in test_spectrum.cpp for the hand-filled slot tests (AccessorsAreTheDocumentedSlots, MutableViewWritesThrough, SizeAndBinCount; the transform-backed tests stay float/double until 3b). For float/double the power() body is textually the same expression in the same type, so the fingerprints below are unaffected.

2. should-fix (docstring): power() does not say that no one-sided factor is applied.
Interior bins return re*re + im*im, DC/Nyquist return dc()^2/nyquist()^2, and nothing says whether the interior is doubled or whether 1/N is applied. Parseval users will guess. Add to the power() doc: "This is |bin[k]|^2 exactly as stored: no factor 2 for the one-sided packing at 1 <= k < N/2 and no 1/N. Over this packing Parseval reads sum_j x[j]^2 = (1/N) * (power(0) + power(N/2) + 2 * sum_{k=1}^{N/2-1} power(k))."

3. should-fix (test gap): the write path is never pinned against a real transform.
MutableViewWritesThrough checks buffer bytes only. pvoc's synthesis pack (synthesis.im(j) -= ...; // conjugate back) relies on the inverse consuming im(k) with the native +i sign. Add one typed test: zero buffer, im(5) = +N/2 via the mutable view (nothing else), inverse_inplace + 2/N, expect sin(2*pi*5*j/N); and re(5) = N/2 → cos. That pins the sign convention on the side pvoc writes, not just the side it reads.

4. should-fix (docs): new public header, no README mention.
grep -n spectrum README.md is empty. F8 makes this docstring the canonical home of the packing; a short paragraph under the FFT section pointing at fft/spectrum.h (and noting that fft.h's copy of the packing is to be cross-referenced later, as the PR says) is the CLAUDE.md checklist's item 3 for anything under include/.

5. nit: size() returns N on a type called spectrum.
Documented ("the number of packed values"), consistent with span semantics, but for (k = 0; k < s.size(); ++k) s.power(k) is the obvious misuse and asserts only in debug. Consider packed_size()/transform_size(), or keep and accept.

6. nit: constructor precondition n >= 2 vs fft.h's >= 4, power of two.
Deliberately looser (N = 2 is DC + Nyquist with an empty interior). Fine; one sentence saying the view does not require a power of two would stop the next reader from "fixing" it.

7. nit: lifetime is implied, not stated. "Non-owning" is in the brief; the docstring should say the view is valid only while the viewed buffer is, and that copying the view (including the mutable-to-const conversion) copies a pointer and a size, not the spectrum. Verified the conversion compiles mutable -> const and is rejected const -> mutable (1 error).

8. nit: [[nodiscard]] is new to include/ and STYLE.md is silent on it. Not a violation; flagging for the process reviewer so the taphouse copy of STYLE.md gets a line rather than the attribute arriving piecemeal.

9. nit: the explicit CTAD guide is redundant with the implicit guide from packed_spectrum(Sample*, std::size_t); harmless. k_tolerance's primary template is unused. AccessorsAreNoexceptAndConstexpr cannot fail at runtime (SUCCEED()), same pattern as test_nn.cpp:234; acceptable.

10. note (no action): -0.0 handling. bin_engineering() negates in Sample before pvoc widens to double; -(-0.0) = +0.0, and atan2(+0, re<0) is +pi where atan2(-0, re<0) is -pi. The pre-PR code also negated before atan2, so behavior is unchanged (and the fingerprints agree).

Checks that passed

• Preconditions vs call sites. re/im/bin_engineering assert 1 <= k < N/2, power asserts k <= N/2, both matching their @pre. Every migrated site stays inside: pvoc analysis/synthesis k, j in [1, m_bins-2] = [1, N/2-1]; compute_envelope k in [1, N/2-1]; log_mel k in [1, N/2-1] plus peeled power(0)/power(N/2); test_pvoc k in [1, num_bins-2].
• Bit identity, reproduced independently. Own FNV-1a-64 harness (pvoc N=1024, 48 000 samples, xorshift32 + two tones; log_mel default geometry, 300 frames), compiled against origin/main's headers and the branch's, linking the same Ooura objects: pvoc {float,double} x ratio {1.5 off, 1.5 on, 1.0 on, 0.7 on} and log_mel {float,double} x {log, PCEN} — 10 cases, identical under gcc -O2 -march=haswell (default -ffp-contract=fast), gcc -ffp-contract=off, gcc -O3 -march=native, gcc -O0 asserts on, clang -O2 -march=haswell (default on), clang -ffp-contract=fast, clang -O3 -ffp-contract=off. The harness is contraction-sensitive (main-vs-main fingerprints differ between fast and off), so agreement is meaningful. Site inspection: re*re + im*im order kept; LPC magnitude still squares in double after the cast; the conjugation moved from -(double)x to (double)(-x), exact in both; no new temporaries that add contraction candidates (the gcc loop body is 2 loads, vmulss, vfmadd132ss, store in both trees).
• Codegen (gcc 13 -O2 -march=haswell -DNDEBUG, instruction count / symbol bytes, main -> branch): run_frame<float> 476 -> 469 (2060 -> 2022 B); run_frame<double> 466 -> 483 (2012 -> 2066 B); log_mel::analyze<float> 232 -> 236; analyze<double> 248 -> 255; compute_envelope unchanged in both. clang 18: run_frame<float> 609 -> 614, <double> 602 -> 603, analyze 429 -> 437 / 435 -> 442. The power() edge branches are hoisted out of the log_mel loop by VRP (one extra prologue branch for the N < 2 case the compiled-out assert would have excluded). Vectorization: both trees unvectorized at -O2 (very-cheap cost model refuses the alias versioning), both vectorized with 16/32-byte vectors at -O3; no regression. Deltas are within +-4% and not a numerics matter.
• Tests. Sign of im(k) for an on-bin sine (+N/2 native, -N/2 engineering), DC and Nyquist slots, and power() at both edges are pinned against real basic_real_fft forward transforms, typed over float/double; the slot test is hand-filled by design. The constexpr battery also compiles with asserts on (no NDEBUG) under both compilers.
• Stage 0 (#20) merge. Merging claude/wave1-stage0-counter into this branch in a scratch clone is a clean auto-merge (the two PRs touch disjoint regions of pvoc.h); the merged tree builds with -DTAP_DSP_WERROR=ON and passes 179/179, including both ClockWrap* tests and the 15 packed_spectrum tests.
• Style. Banner, ////@pre docs, member order, west-const, braces: conform. Docstring carries bin[k] = a[2k] + i a[2k+1], DC a[0], Nyquist a[1], W = exp(+2*pi*i/N), unnormalized inverse with 2/N.

Verdict

No blocker; mergeable once 1 (arithmetic-agnostic view, or at minimum the power_type and bin_engineering constraint so Stage 3 does not reopen a fresh header), 2 and 3 land; 4 can ride the docs PR if the process reviewer prefers.

---

Generated by Claude Code

[tap]

Fix pass: c609c5e (+ 5631d35, a one-line MSVC C4310 fix in the new promotion test, caught by the Windows leg)

Thanks to both reviewers; every finding is listed below with what happened to it. gcc 13.3 and clang 18 with -DTAP_DSP_WERROR=ON: zero warnings, 191/191 (31 packed_spectrum tests). clang-format and tidy clean. The twelve fingerprints are identical to the pre-PR values (table in the PR body, now reproducible from tools/fingerprint/).

Review A (correctness)

#	finding	outcome
1	static_assert forecloses the fixed-point views; naive relaxation is wrong	Applied. Value types are now float, double, int16_t, int32_t. power_type = conditional_t<is_integral_v<value_type>, int64_t, value_type>; power() promotes before the multiply (the float/double bodies keep the same type and operand order, hence the unchanged fingerprints). bin_engineering() has requires is_floating_point_v<value_type>. The int64 corner re == im == INT32_MIN is documented on power_type as excluded by the fixed-point scaling, not guarded. int16_t/int32_t join the hand-filled suite (packed_spectrum_slot_test: slots, write-through, deduction, size), and PowerPromotesBeforeTheMultiply pins your exact example (int16 pair of 30000 → 1 800 000 000, and 2^30 pairs → 2^61).
2	power() doc: no one-sided factor 2, no 1/N, Parseval formula	Applied, with the formula as you wrote it, and ParsevalHoldsOverThePacking pins it against a random signal (float and double).
3	write path never pinned through a real inverse	Applied. WritesThroughTheViewInvertToTheDocumentedTones: zero buffer, im(5) = +N/2 through the mutable view, inverse_inplace + 2/N → sin(2π·5·j/N); re(5) = N/2 → cos.
4	README mention	Declined for this PR, recorded. Per the process decision (review B #6): a view is not a primitive, so no README section; the FFT "Key contract points" should gain one pointer line in the wave-2 fft.h/README pass. Recorded under follow-ups in the PR body.
5	size() returns N on a type called spectrum	Kept and accepted. Span semantics, documented; num_bins() is the bin count and the migrated loops use it.
6	n >= 2 vs fft.h's >= 4, power of two	Applied. The @pre now says it is deliberately looser (even N only, N = 2 is DC + Nyquist with an empty interior) and that the view does not require a power of two.
7	lifetime implied, not stated	Applied. Class docstring states non-owning semantics: pointer + size, valid only while the buffer is, copies (the const conversion included) copy the pointer, never the spectrum.
8	[[nodiscard]] new to include/, STYLE.md silent	No action here, flagged in the PR body for the taphouse maintainers.
9	explicit CTAD guide redundant; unused k_tolerance primary; SUCCEED() test	Guide dropped (DeductionFollowsThePointer still passes on the implicit guide). The k_tolerance primary is the test_fft.cpp pattern and stays; the constexpr test keeps the test_nn.cpp pattern.
10	-0.0 note	No action, as you say: unchanged behavior, fingerprints agree.

Review B (process / downstream)

#	finding	outcome
1	log_mel.h:35 still says "(Ooura contract)"	Applied. Now "(the packed spectrum defined in fft/spectrum.h)". Comment-only; fingerprints unchanged.
2	tests/CMakeLists.txt conflicts with #22; alphabetical placement	Applied. test_spectrum.cpp is now between test_sample_traits.cpp and test_yin.cpp. Merge order after #22 acknowledged in the PR body; the resolution is keeping both lines.
3	new suite invisible to the QEMU legs under #17's positive filter	Not this PR's files. Recorded as a follow-up in the PR body; Part 13 turns the filter negative, which resolves it.
4	fingerprint evidence not reproducible from the repository	Applied. tools/fingerprint/ is a standalone CMake project like tools/capi (root CMake untouched) that prints the twelve FNV-1a-64 lines for pvoc (float/double × ratio 1.0/1.5 × formant off/on) and log_mel (float/double × log/PCEN) over the fixed xorshift32 corpus. Its header says the hashes are host + compiler + flags + libm specific, are diffed across two trees on one host, and are never committed as golden values or asserted in a test. Built at the PR head it prints exactly the PR body's table. This is the tool the Stage 6 gate presupposes (Part 13).
5	MuTap deferral stated but not tracked	Applied. tap/MuTap#51: the four headers with site counts (fdaf.h 25, fd_kalman.h 28, nn_suppressor.h 15, postfilter.h 9), the per-header gate (fingerprint identical + 0% icount), and the rule that hot loops use native accessors only, never bin_engineering(). Linked from the PR body.
6	fft.h and README pointers	Recorded as wave-2 follow-ups in the PR body, with your README decision (no section; one line under "Key contract points"). Not edited here.
7	[[nodiscard]] house-style question	No action here, flagged for the taphouse maintainers in the PR body.
8	PR body precision (CMake line; tidy subset)	Applied. The body now says the test_yin.cpp) line is split to insert the entry, and calls the local three-file tidy run a subset of CI's full scan.

🤖 Generated with Claude Code

https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy