fix(FpySequencer): cache debug deserialization to prevent repeated WARNING events

[philphauler]

Related Issue(s)	#5661
Has Unit Tests (y/n)	y
Documentation Included (y/n)	n
Generative AI was used in this contribution (y/n)	AI

---

Change Description

Caches the last-warned statement index in updateDebugTelemetryStruct() so a persistent deserialization failure emits WARNING_HI once instead of repeating on every telemetry tick.

Rationale

While paused, updateDebugTelemetryStruct() calls deserializeDirective() on every telemetry tick. If the directive fails, a WARNING_HI event repeated at the telemetry rate flooded the event log and masked other issues.

Testing/Review Recommendations

Three new cases in FpySequencerTestMain.cpp (596c618): tlmWriteDebugCachesFailedDeserialize (one DirectiveDeserializeError, later ticks on the same index emit nothing), tlmWriteDebugCacheKeyedOnStatementIndex (a different index deserializes again), tlmWriteDebugCacheClearedOnRuntimeReset (after resetRuntime a new sequence at index 0 is tried again). Run locally with and without the fix: with it 3/3 pass, without it 3/3 fail.

Future Work

None planned.

AI Usage (see policy)

Claude Code was used for codebase navigation (locating the telemetry-tick call site) and fix design (the caching approach).

IAMAI

[sylvesterkaczmarek]

The failed-statement cache survives resetRuntime(): that action reconstructs m_runtime but does not invalidate m_debug.cachedStmtIndex/nextStatementReadSuccess. If one sequence fails debug deserialization at index 0 and a new sequence starts at index 0, telemetry can keep returning early without ever trying the new statement. Clear the cached failure on runtime/sequence reset and add a two-sequence regression.

[LeStarch]

@zimri-leisher would you review or close this PR? I am not enough informed to know if this is a good change.

[zimri-leisher]

Needs tests @philphauler

[philphauler]

Onnit 🫡⚡️

[philphauler]

Hey @zimri-leisher, hey @sylvesterkaczmarek 👋 thanks to both of you, the reset-runtime catch was a good one and made the tests better.

Tests are in 596c618, three cases in FpySequencerTestMain.cpp:

• tlmWriteDebugCachesFailedDeserialize: a failed deserialize at index N emits DirectiveDeserializeError once, later tlmWrite calls on the same index emit nothing.
• tlmWriteDebugCacheKeyedOnStatementIndex: a different index deserializes again.
• tlmWriteDebugCacheClearedOnRuntimeReset: the two-sequence case, after resetRuntime a new sequence at index 0 is tried again and the event fires again.

Ran them with and without the fix: with it 3/3 pass, without it 3/3 fail, each on the same assertion (DirectiveDeserializeError history size 2, expected 1).

One small thing I noticed and left alone: the debug path logs currentStatementIdx(), which is nextStatementIndex minus one, so DirectiveDeserializeError reports the previous statement's index. Pre-existing, happy to file it separately if you want.

Have a great day! ⚡

[sylvesterkaczmarek]

Re-reviewed the cache invalidation and tests. resetRuntime() now invalidates the cached statement index, so a new sequence starting at the same index is deserialized again rather than inheriting the previous sequence’s failure. The added two-sequence regression directly covers the case I raised. No further blocking issue from my review.

[philphauler]

@sylvesterkaczmarek Looking back at this though, I still think there is too much locs.. gotta be more optimised, imma tweak 🫡⚡️

[LeStarch]

@zimri-leisher apart from the missing tests, is this the right approach?

[zimri-leisher]

@philphauler can we do this without new state? for instance, we could maybe just telemeter down the deserialization status and pass an arg with a default value that suppresses warning generation by deserializeDirective?

[lestarch-autobot]

Hello @philphauler, and thank you for contributing to F´!

This pull request is signed IAMAI, indicating it was produced by an AI agent. Per the contributor guidelines, all code contributions begin with an issue that is reviewed and approved by the Change Control Board (CCB) before implementation work starts. This gate exists to keep limited maintainer review time focused on work that is in scope for F´.

At the moment the linked issue(s) have not yet been approved by the CCB: #5661 (CCB Resolution: not yet set).

Recommendation to maintainers: close this PR for now, and reopen it once the CCB has weighed in on the linked issue(s). If the CCB approves, the work here can be picked back up immediately.

This is an automated notice from lestarch-autobot. It is a process note, not a judgment on the quality of the contribution.

[philphauler]

Hi zimri 👋

You're right — the parameter approach is cleaner. Replaced the cached-state version with an emitWarnings default-arg on deserializeDirective: the debug telemetry path passes false, no new state, the dispatch path is unchanged, and the result is already visible through Debug_NextStatementReadSuccess.

Pushed as f302c07. FpySequencer UT on devel: 299/299 pass. The fix is a compile-time-visible guard (if (emitWarnings) wrapping every log_WARNING_HI_DirectiveDeserializeError call), so the divergence is structural rather than test-observable: the debug telemetry path no longer emits the event on each tick, but the dispatch path is unchanged.

Have a nice week 🙏

[sylvesterkaczmarek]

Thanks for simplifying this. On f302c07, removing the cache eliminates the stale-state issue I raised, and normal dispatch retains the default warning behavior. Could you retain a focused regression for the replacement: repeated debug telemetry on a malformed directive should report failure without emitting DirectiveDeserializeError, while dispatching that directive should still emit the warning? The current diff has no test covering that distinction.

[philphauler]

I can run more tests for sure, and look deeper 🙌⚡️ also @sylvesterkaczmarek would you like to connect perhaps through email or linked in?

[sylvesterkaczmarek]

Happy to connect on LinkedIn, thanks! And I appreciate you taking another look at the regression coverage.

[zimri-leisher]

I appreciate the work, but again I'm just not sure this is the right way to do this. I haven't really thought very hard about this issue though. For now I think I'd like to put this issue on pause until I have more time to review. It's not a pressing issue.

[philphauler]

For sure @zimri-leisher if you wanna ever get back to it just ping, or just throw whatever at me, and I'll do my best to figure it out 🚀

[philphauler]

Happy to connect on LinkedIn, thanks! And I appreciate you taking another look at the regression coverage.

@sylvesterkaczmarek just saw this, and followed you on linked in 🙏🙌