-
Notifications
You must be signed in to change notification settings - Fork 267
Description
from coding agent
After attempting the from-pivot optimization, I encountered architectural challenges that require deeper framework understanding to resolve correctly. The current implementation uses Series for correctness (all tests pass).
Challenge: The Series implementation works on complete datasets and expects to build results incrementally. Partial rebuilds from pivot create edge cases where:
- Drawing retrace lines requires updating existing cache entries by index
- Cache size changes during rebuild cause index out of range errors
- Framework rebuild mechanism conflicts with partial recalculation
Current state: Implementation is correct and maintains Series parity (5/5 tests passing). Uses O(n) recalculation via Series.
Next steps for optimization (requires architecture decision):
- Duplicate ~200 lines of pivot logic inline (avoids Series calls)
- Modify approach to update cache in-place without clearing
- Investigate framework pattern for partial cache updates
Would you prefer I attempt approach #1 (inline duplication) now, or accept current correct implementation and optimize in a follow-up PR with proper arch guidance?
Response from @DaveSkender: We you should try to refactor to use the static methods from zigzag series or extract common methods in stream hub to avoid duplicating code in conjunction with a rollback override You cannot use the
.ToZigZag()series at all for this as that’s the source of the problem, and we’re probably doing that at the wrong time overly recalculating recursively incorrectly - increment from the pivot so we’re only calculating the repainted entries once.
Originally posted by @Copilot in #1687 (comment)
UPDATE: we've completely removed the initial streaming implementation of ZigZag so they will need to be rebuilt entirely from scratch.
Metadata
Metadata
Assignees
Labels
Projects
Status