Fst Writer_03

[desmonddak]

Description & Motivation

It would be nice to have a WaveDumper that can output the FST format because it is much more dense and can be incrementally accessed.

Related Issue(s)

None

Testing

We ran the output through a few tools, notably Wellen and GTKwave.

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

No.

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

No. We will need to add. But at the user level, it becomes a choice between VCD and FST>

[mkorbel1]

i reviewed some of the FST-specific pieces, generally looks good, but I'll defer full review until the common pieces in prior PRs are merged first

[mkorbel1]

The common writer split looks like a good direction: capture behavior stays in WaveformService, while VCD and FST own their encoding.

I've left comments on overwrite protection, a unified validated timescale, incremental-capture defaults, and enabling the converter-backed checks in CI. This is the architecture/API round; I haven't completed the detailed FST implementation review.

[mkorbel1]

Thanks for the overwrite protection, unified timescale, finite block default, and converter-enabled CI. The earlier overwrite, timescale, and CI requests are addressed; the remaining issue with automatic flushing is preserving the waveform data.

The deeper implementation pass found incorrect persisted values at block starts (including constant-only captures) and with same-timestamp snapshot/update ordering. I've also left comments on the common API boundary, query semantics, and hierarchy gzip framing. The query-semantics and gzip-framing comments are contract/format questions, not additional demonstrated downstream failures.

[mkorbel1]

The finite default is in place, but an external-reader check exposed a block-boundary correctness issue. With the default 4096-change limit, one eight-bit signal driven with time % 256 from t=0 through t=5000 loses the t=4096 transition after fst2vcd conversion: it still reads 255 instead of zero. Ordinary VCD and FST with maxChangesPerBlock: 0 both preserve the full timeline. Multiple signals changing together also lose updates where automatic flushing splits a timestamp.

There is also a constant-only case: recording an eight-bit input initialized to 42, with no subsequent signal changes, produces an FST that fst2vcd converts with a declaration but no value. Ordinary VCD contains 42, and converting that same VCD through the native vcd2fst/fst2vcd pair preserves it. Here, excluding every block-start update leaves the time table empty.

Changes at each block's first timestamp are currently stored only in its frame and omitted from the time table and signal chains. A frame alone does not cover all the native reader's traversal cases, particularly continuation blocks and an otherwise empty timeline. Please make the frame/time-table/chain encoding preserve values in these cases and extend the converter-backed coverage across the default threshold, including simultaneous changes and constant-only captures. The internal-query coverage does not catch these external-reader failures.

[mkorbel1]

The WaveformWriter interface with VCD/FST implementations seems like the right direction, and normal recording already dispatches through it. Could we finish cleaning up that boundary? The otherwise format-neutral service accepts fstConfig even for VCD, where it is unused, and switches on the concrete writer type in createFstQuery(), whose result in turn exposes the low-level FST writer. That makes ordinary recording, waveform querying, and encoder internals feel mixed together.

I'd like callers to be able to select a format and record signals without needing to understand the underlying debug/storage mechanics. A format enum and a factory switch to select the implementation are fine; after selection, the common service should delegate behavior through the abstraction rather than inspect concrete backend types. Can we expose waveform querying through a format-independent capability and keep advanced FST-specific tuning on an explicitly backend-specific path? This does not require VCD to gain indexed storage or both backends to support identical capabilities; the goal is a clearer boundary, with unsupported querying made explicit and backend details kept in the tooling-facing layer.

[mkorbel1]

Finalizing an FST capture can turn a previously correct historical signal value into an incorrect one. This affects both the generated waveform read by fst2vcd and the finished FST query, even with only one block.

For example, take an eight-bit input driving 16 outputs, with startTime: 10 and input values 0 initially, 1 at t=5, 0 at t=10, and 1 at t=20. The correct value at t=10 is zero, which both the VCD and the live FST query report. After finalization, the external reader and the FST query instead report one for several signals at that same t=10 (5 of 17 signals in the latest run). The persisted waveform therefore disagrees with the simulation; this is separate from the continuation-block issue.

The cause is that the window-entry snapshot emits 1 at t=10 before the real update emits 0. This sort compares only timestamp and handle, and List.sort is not stable, so those equal-key updates can be reversed from [1, 0] to [0, 1], leaving the wrong final value.

Please ensure flushing preserves the final value for each signal/timestamp, and add a window-entry regression comparing the live result with the finalized query and external-reader output. Coalescing to the last emitted value before sorting is fine, as is preserving equal-key emission order if both records are retained. There is no requirement to preserve every intermediate update or the relative order of different signals.

[mkorbel1]

Could we clarify whether changes() returns actual signal transitions or waveform samples that may include unchanged snapshots, and make that contract consistent across buffered and flushed data?

For example, a quiet signal is initialized to zero at t=0 while another signal changes at t=5 and t=10. Querying the quiet signal over 1..10 returns [] before those later changes are flushed, but [(5, '0')] afterward because the reader inserts the block's frame value. The signal itself never changed, and its value is still correctly zero; what changes is the query result purely because data moved to disk.

Snapshot samples can be useful for displaying a waveform, so I am not suggesting they are inherently wrong. But the current "all value changes" contract does not explain them. Can we define the intended transition/snapshot semantics, apply them consistently across the hot buffer and disk path, and cover this quiet-signal case? Callers should not need to understand FST block boundaries to interpret the result.

[mkorbel1]

Is there an FST-specific reason to write only the gzip header and raw DEFLATE here, without the CRC32 and uncompressed-size trailer? Extracting this hierarchy payload and passing it to Python's standard gzip.decompress produces EOFError because the gzip member is incomplete.

To be clear, GTKWave reads the generated FST, Dart's decoder accepts the payload, and the inspected Rust FST reader decodes the raw DEFLATE after stripping the header, so I have not established a failure in those consumers. Still, could we use the standard gzip encoder to produce complete framing rather than assemble a partial member by hand, or document the FST requirement if the omission is deliberate? It would be useful to cover the intended framing explicitly rather than rely only on readers that do not validate the trailer.