Skip to content

Review follow-up: changes required on PR #84 #87

Description

@BnJam

Phase 2 review of PR #84 (src/trends.rs recursion logic) requested changes before merge.

Required changes:

  • Major: spike/outlier split points are silently dropped (IEEE-754 simulation: spike series yields segments [0,24] + [26,49], index 25 missing), contradicting the PR's validation claim. Fix so the split index is preserved.
  • Major: recursion depth is still O(n) worst-case (n=10k convex series -> depth ~4052); large inputs can overflow the stack. Convert to an explicit stack/iterative approach or add a depth guard.
  • Minor: 2-element input with threshold=0.0 returns zero segments.
  • Minor: no new regression tests; existing test only checks len >= 2. Add tests for the spike and deep-recursion cases.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions