issue/4213/fix/timelines-treat-0s-as-number

[lsabor]

closes #4213
fix group timelines treating 0s as null

Summary by CodeRabbit

• Bug Fixes
  • Charts now treat zeros and empty values as valid data when present, preventing unintended omissions.
  • Cursor timing and default position logic improved for more consistent display across closed/open data points.
  • Line and point visibility rules refined to reduce flicker and ensure accurate rendering of series.
  • Timestamp, resolution, and boundary selection logic hardened to prefer explicit close timestamps when available.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19945f2 and 32e53c1.

📒 Files selected for processing (2)

• front_end/src/components/charts/group_chart.tsx
• front_end/src/components/charts/numeric_timeline.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• front_end/src/components/charts/group_chart.tsx

---

📝 Walkthrough

Walkthrough

Replaced numerous truthiness checks with explicit nil (null/undefined) checks in chart components, altering cursor/default-timestamp selection, line/point filtering, isClosed/resolution logic, and public data construction so falsy-but-valid values (e.g., 0, "") are treated as present data.

Changes

Cohort / File(s)	Summary
Group Chart Logic front_end/src/components/charts/group_chart.tsx	Replaced truthiness checks with explicit nil checks (isNil) for fields like lastLineX, cursorTimestamp, closeTime, openTime, actualCloseTime, userValue, and CP data. Updated default cursor selection, line/point filtering and visibility, isClosed computation, resolution/resolveTime fallbacks, and domain timestamp selection.
Numeric Timeline front_end/src/components/charts/numeric_timeline.tsx	Changed early-exit conditions in useMemo to use explicit isNil checks for resolution, resolveTime, and actualCloseTime so non-nil falsy values (e.g., 0, empty string) proceed through logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• ncarazon

Poem

🐰 I hopped through code with careful checks,
Nil not false, no phantom wrecks.
Zeros now sparkle, timelines mend,
A rabbit's nod — the view's well penned. ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'issue/4213/fix/timelines-treat-0s-as-number' clearly describes the main change: fixing timeline handling to treat zeros as numbers rather than null values, directly addressing the primary objective of the PR.
Linked Issues check	✅ Passed	The PR code changes directly address issue #4213 by replacing falsy checks with explicit nil checks (isNil), ensuring zero values are treated as valid numbers rather than null, fixing the timeline rendering bug.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing nil-check logic in timeline components (group_chart.tsx and numeric_timeline.tsx) to treat zeros as numbers, which is directly within the scope of the linked issue.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch issue/4213/fix/timelines-treat-0s-as-number

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

front_end/src/components/charts/group_chart.tsx (1)

709-712: Suggest adding regression tests for zero-valued forecasts.

The fix is comprehensive, but there's no test coverage for the scenario that triggered the bug: a user or CP forecast value of exactly 0. Without at least one snapshot or unit test, a future refactor could silently reintroduce the truthiness check.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front_end/src/components/charts/group_chart.tsx` around lines 709 - 712, Add
a regression test that constructs a choiceItems/forecast scenario containing a
forecast value of exactly 0 and verifies the component logic (e.g., the
closeTimes mapping/filtering and latestTimestamp computation) treats 0 as a
valid value rather than falsy; create a unit or snapshot test that renders the
GroupChart (or the function that computes closeTimes/latestTimestamp) with a
choiceItems entry whose closeTime or forecast is 0, assert the output includes
that zero-valued forecast and that any derived timestamp/latestTimestamp is
computed correctly, and commit the test so future refactors don’t reintroduce
truthiness-based filtering of zero.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@front_end/src/components/charts/group_chart.tsx`:
- Around line 709-712: Add a regression test that constructs a
choiceItems/forecast scenario containing a forecast value of exactly 0 and
verifies the component logic (e.g., the closeTimes mapping/filtering and
latestTimestamp computation) treats 0 as a valid value rather than falsy; create
a unit or snapshot test that renders the GroupChart (or the function that
computes closeTimes/latestTimestamp) with a choiceItems entry whose closeTime or
forecast is 0, assert the output includes that zero-valued forecast and that any
derived timestamp/latestTimestamp is computed correctly, and commit the test so
future refactors don’t reintroduce truthiness-based filtering of zero.

[github-actions]

Cleanup: Preview Environment Removed

The preview environment for this PR has been destroyed.

Resource	Status
🌐 Preview App	Deleted
🗄️ PostgreSQL Branch	Deleted
⚡ Redis Database	Deleted
🔧 GitHub Deployments	Removed
📦 Docker Image	Retained (auto-cleanup via GHCR policies)

---

Cleanup triggered by PR close at 2026-02-28T01:58:46Z

[cemreinanc]

lgtm

[ncarazon]

LGTM