Skip to content

Fix smoothed binary behavior trends - #2307

Merged
Asherlc merged 4 commits into
mainfrom
Asherlc/issue-2156-discrete-behavior-chart
Jul 29, 2026
Merged

Asherlc merged 4 commits into
mainfrom
Asherlc/issue-2156-discrete-behavior-chart

Conversation

@Asherlc

@Asherlc Asherlc commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • render boolean journal answers as isolated point observations instead of smoothed lines
  • keep numeric journal outcomes as line series in the same chart
  • label boolean values as Yes/No in tooltips and the chart accessibility description
  • expand the Tracking Storybook fixture and TDD coverage for mixed boolean/numeric data

Architecture

  • preserves the existing journal.entries server contract and raw 0/1 observations
  • preserves the separately server-computed behaviorImpact.impactSummary outcome comparison
  • mobile has no Journal trends chart; its existing Behavior Associations screen remains the parity outcome surface and its focused tests pass

Validation

  • pnpm exec vitest --project unit --run packages/web/src/components/TimeSeriesChart.test.ts packages/web/src/components/JournalPanel.test.tsx (16 passed)
  • pnpm test (948 files passed, 14,713 tests passed, 2 files/21 tests skipped)
  • root, server, web, and mobile tsc --noEmit
  • web production build
  • web Storybook production build
  • mobile Behavior Associations test (7 passed)
  • code and policy lint passed; local SQLFluff could not obtain ClickHouse because Docker exhausted its predefined network pools, so the isolated CI analytics gate remains required before merge

Fixes #2156

Summary by Sourcery

Render boolean journal trend data as discrete Yes/No points alongside numeric line series and improve chart accessibility and test coverage.

New Features:

  • Support mixed point (boolean) and line (numeric) series in the shared time series chart component.
  • Add ARIA descriptions for time series charts summarizing how each series is visualized.

Enhancements:

  • Map boolean journal questions to point visualizations with Yes/No value formatting in the trends view.
  • Extend the Tracking Storybook journal fixture to include both boolean and numeric observations.

Documentation:

  • Add a TDD plan document describing the discrete behavior chart behavior, scope, and validation strategy.

Tests:

  • Add unit tests verifying point vs line series rendering, tooltip formatting, and accessibility metadata for time series charts.
  • Add JournalPanel tests covering mixed boolean/numeric data classification into chart series.

Summary by cubic

Render boolean journal answers as discrete Yes/No points instead of smoothed lines; numeric outcomes remain line series. Adds chart-level ARIA descriptions and clearer tooltips to avoid misleading trends.

  • Bug Fixes
    • Show boolean observations as point series with Yes/No tooltip formatting and a chart-level ARIA description summarizing each series.
    • Keep numeric questions as smoothed line series; mixed boolean/numeric charts are supported.
    • No server contract changes; journal.entries and behaviorImpact.impactSummary are unchanged. Mobile UI is unaffected.
    • Update tests and Storybook fixture for mixed data; correct the documented chart test command.

Written for commit 2ec8833. Summary will update on new commits.

Review in cubic

Copilot AI review requested due to automatic review settings July 29, 2026 20:11
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@codereviewbot-ai

codereviewbot-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown

LGTM!

The implementation cleanly renders boolean journal entries as discrete scatter plot points alongside numeric lines, with proper ARIA accessibility descriptions, tooltip formatting, and comprehensive unit tests.


🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Asherlc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c7bdf4d7-0ed5-4541-9e64-f0f0b30811e7

📥 Commits

Reviewing files that changed from the base of the PR and between 1b86e53 and 2ec8833.

📒 Files selected for processing (7)
  • docs/superpowers/plans/2026-07-29-discrete-behavior-chart.md
  • packages/web/src/components/JournalPanel.stories.tsx
  • packages/web/src/components/JournalPanel.test.tsx
  • packages/web/src/components/JournalPanel.tsx
  • packages/web/src/components/TimeSeriesChart.stories.tsx
  • packages/web/src/components/TimeSeriesChart.test.ts
  • packages/web/src/components/TimeSeriesChart.tsx

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

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adjusts journal behavior trends so boolean observations render as discrete Yes/No points with accessible labeling, while numeric observations remain smoothed line series, and expands tests/docs to cover the mixed data behavior.

Sequence diagram for JournalPanel boolean/numeric trend rendering

sequenceDiagram
  actor User
  participant JournalPanel
  participant TimeSeriesChart
  participant DofekChart

  User->>JournalPanel: clickTrendsButton()
  JournalPanel->>JournalPanel: buildSeriesFromEntries()
  JournalPanel->>JournalPanel: setSeriesVisualization(point|line)
  JournalPanel->>JournalPanel: setAccessibilityDescriptionAndFormatValue()
  JournalPanel->>TimeSeriesChart: TimeSeriesChart({series})
  TimeSeriesChart->>TimeSeriesChart: buildAccessibilityDescription()
  TimeSeriesChart->>TimeSeriesChart: mapSeriesToScatterOrLine()
  TimeSeriesChart->>DofekChart: DofekChart({option})
  DofekChart-->>User: renderChartWithPointsAndLines()
Loading

File-Level Changes

Change Details Files
Introduce visualization and accessibility metadata to TimeSeriesChart series and render boolean data as scatter points with ARIA descriptions and Yes/No tooltip formatting.
  • Extend TimeSeriesChart Series type with accessibilityDescription and visualization fields and include them in seriesFormatters and chart option construction.
  • Add ECharts aria configuration that builds a combined chart description from per-series accessibilityDescription or visualization kind.
  • Map series with visualization="point" to dofekSeries.scatter with fixed symbol size, and default others to dofekSeries.line with existing smoothing/area behavior.
  • Add a unit test that passes mixed boolean and numeric series into TimeSeriesChart and asserts scatter vs line series, ARIA description text, and Yes/No tooltip formatting.
packages/web/src/components/TimeSeriesChart.tsx
packages/web/src/components/TimeSeriesChart.test.ts
Update JournalPanel trends to treat boolean questions as point series with Yes/No formatting while still charting all numeric-backed answers, and capture TimeSeriesChart props in tests.
  • Relax journal trends filtering to consider any question with non-null answer_numeric as chartable, including boolean 0/1 answers.
  • When building chart series, tag boolean questions with accessibilityDescription describing Yes/No points, visualization="point", and a formatValue mapping 1→Yes and 0→No; keep numeric questions as visualization="line".
  • Extend JournalPanel tests with a TimeSeriesChart mock that records props, and reset captured props before each test.
  • Add a regression test that feeds mixed boolean and numeric entries/questions, navigates to Trends, and asserts the produced series data, visualization type, accessibility description, and Yes/No formatting.
packages/web/src/components/JournalPanel.tsx
packages/web/src/components/JournalPanel.test.tsx
Adjust JournalPanel Storybook tracking fixture to include mixed boolean/numeric journal data consistent with the new chart semantics.
  • Add a second alcohol journal entry with a 0-valued boolean observation to the entries fixture.
  • Introduce an alcohol boolean question definition and reorder energy to follow alcohol via sort_order values, creating a mixed boolean/numeric trends scenario.
  • Ensure the fixture exercises the new point vs line chart behavior in Storybook.
packages/web/src/components/JournalPanel.stories.tsx
Document the discrete behavior chart TDD plan, rationale, and test strategy for rendering boolean observations as points instead of smoothed lines.
  • Add a markdown plan describing the goal of avoiding invented fractional behavior values when charting boolean data.
  • Capture current evidence about server contracts and TimeSeriesChart smoothing behavior leading to misleading fractional curves.
  • Outline unit, component, Storybook, and mobile parity test strategies and file-level tasks for implementing the change.
  • Record completed tasks and remaining steps for the associated Fixes [TRACK-03] Binary behavior answers smoothed into impossible fractional lines #2156 work item.
docs/superpowers/plans/2026-07-29-discrete-behavior-chart.md

Assessment against linked issues

Issue Objective Addressed Explanation
#2156 Render binary (boolean) journal behavior answers (e.g., caffeine, alcohol, meditation) as discrete observations instead of smoothed lines that imply impossible fractional values.
#2156 Visually separate and clearly represent outcome observations (behavior impact / readiness outcomes) from discrete behavior events in the tracking UI, per the recommendation to overlay outcome observations separately. The PR explicitly preserves the existing behaviorImpact.impactSummary outcome comparison and does not change how outcome observations are overlaid or visually separated in the tracking chart. Changes focus solely on rendering boolean journal answers as point observations and improving their accessibility, without modifying outcome visualization.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Render boolean journal trends as discrete Yes/No points (no smoothing)

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Render boolean journal observations as discrete scatter points to avoid fractional smoothed
 trends.
• Keep numeric journal outcomes as smoothed line series in the same chart.
• Add Yes/No tooltip + ARIA descriptions and expand Storybook + unit coverage for mixed data.
Diagram

graph TD
  API[("journal.entries API")] --> JP["JournalPanel"] --> TSC["TimeSeriesChart"] --> EC["ECharts via Dofek"]
  SB["Storybook fixture"] --> JP
  UT["Unit tests"] --> JP --> TSC
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Disable smoothing for boolean series (still a line)
  • ➕ Minimal rendering change; keeps line-only chart implementation.
  • ➖ Still visually implies continuity between observations; can suggest values persisted across unrecorded days.
2. Step line for boolean series
  • ➕ Clearly avoids fractional values while keeping a line representation.
  • ➖ Strongly implies the previous answer remains true until the next data point; still invents state between observations.
3. Split boolean and numeric into separate charts
  • ➕ Avoids mixed visualization types; simpler per-chart semantics.
  • ➖ More UI space and cognitive load; harder to compare behaviors against numeric outcomes at a glance.

Recommendation: Keep the PR’s scatter-point approach for boolean observations. It best preserves the “observations only” semantics (no implied continuity) while still allowing numeric series to remain a line in the same comparative chart. The added Yes/No formatting and ARIA description support makes the mixed visualization explicit and testable.

Files changed (6) +327 / -17

Enhancement (1) +28 / -4
TimeSeriesChart.tsxSupport point (scatter) series and generate chart-level ARIA descriptions +28/-4

Support point (scatter) series and generate chart-level ARIA descriptions

• Extends the Series model with visualization + accessibilityDescription and builds ECharts options that render point series via scatter (symbolSize 10) while preserving existing line behavior; adds an ARIA label description summarizing each series’ visualization.

packages/web/src/components/TimeSeriesChart.tsx

Bug fix (1) +13 / -4
JournalPanel.tsxMap boolean questions to point series with Yes/No formatting and a11y text +13/-4

Map boolean questions to point series with Yes/No formatting and a11y text

• Updates trend-series construction to treat boolean data_type as chartable numeric storage, emitting point visualization series with an accessibility description and value formatter that returns Yes/No.

packages/web/src/components/JournalPanel.tsx

Tests (2) +185 / -8
JournalPanel.test.tsxTest boolean-vs-numeric series classification passed into TimeSeriesChart +114/-8

Test boolean-vs-numeric series classification passed into TimeSeriesChart

• Enhances the TimeSeriesChart mock to capture props and adds a regression test asserting boolean questions become point series with Yes/No formatting and numeric questions remain line series.

packages/web/src/components/JournalPanel.test.tsx

TimeSeriesChart.test.tsAdd mixed scatter/line rendering, tooltip, and ARIA description tests +71/-0

Add mixed scatter/line rendering, tooltip, and ARIA description tests

• Adds a unit test verifying point series become ECharts scatter with symbol sizing, line series remain smoothed lines, ARIA descriptions summarize visualization per series, and tooltips format boolean values as Yes/No.

packages/web/src/components/TimeSeriesChart.test.ts

Documentation (1) +79 / -0
2026-07-29-discrete-behavior-chart.mdAdd TDD plan for discrete boolean behavior chart semantics +79/-0

Add TDD plan for discrete boolean behavior chart semantics

• Introduces a written plan describing why boolean trends must be rendered as discrete points, the expected Yes/No semantics, and the test/validation strategy across web and mobile parity concerns.

docs/superpowers/plans/2026-07-29-discrete-behavior-chart.md

Other (1) +22 / -1
JournalPanel.stories.tsxExtend Tracking Storybook fixture with boolean Alcohol observations +22/-1

Extend Tracking Storybook fixture with boolean Alcohol observations

• Adds a boolean journal entry and corresponding question metadata to the fixture, enabling Storybook to exercise mixed boolean (0/1) and numeric trend rendering.

packages/web/src/components/JournalPanel.stories.tsx

@Asherlc

Asherlc commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Regarding the outcome-overlay assessment in the Sourcery reviewer's guide (#2307 (comment)): no additional contract or chart change is needed.

  • The Tracking chart now overlays the two observation kinds separately: server-identified boolean questions are scatter points, while numeric observations remain line series. JournalPanel.test.tsx covers both in one chart.
  • Next-day readiness is not a raw journal observation. It remains the separate server-computed behaviorImpact.impactSummary association surface on both web and iOS, avoiding client-side metric computation.
  • Adding a new daily readiness series to the journal contract would expand the approved strategy and duplicate the existing outcome surface rather than fix the confirmed interpolation bug.

@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Storybook previews for 5477dfa8 are ready:

This comment updates automatically on each PR push.

@qodo-code-review

qodo-code-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 167 rules

Grey Divider


Remediation recommended

1. Missing point TimeSeriesChart story ✓ Resolved 📘 Rule violation ▣ Testability
Description
TimeSeriesChart now supports a visualization: "point" variant, but its Storybook file has no
story that exercises this new rendering mode. This violates the requirement to cover significant UI
states/variants in colocated Storybook stories, and risks regressions for boolean/point charts.
Code

packages/web/src/components/TimeSeriesChart.tsx[R12-21]

interface Series {
  name: string;
  data: [string, number | null][];
+  accessibilityDescription?: string;
  color?: string;
  areaStyle?: boolean;
  yAxisIndex?: number;
  formatValue?: (value: number) => string;
+  visualization?: "line" | "point";
}
Relevance

●●● Strong

Strong precedent: missing Storybook coverage for significant variants/states is routinely requested
and accepted.

PR-#1560
PR-#1197

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR adds a new visualization?: "line" | "point" variant to TimeSeriesChart, but the colocated
stories define only line-based series and never set visualization: "point", so the new visual
variant is not covered as required.

Rule 773538: Colocate Storybook stories with React components and cover key UI states
packages/web/src/components/TimeSeriesChart.tsx[12-21]
packages/web/src/components/TimeSeriesChart.stories.tsx[46-120]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TimeSeriesChart` gained a new visual variant (`visualization: "point"`), but `TimeSeriesChart.stories.tsx` does not include any story that renders a point/scatter series.

## Issue Context
The compliance checklist requires Storybook stories to cover key UI states and significant visual variants for components under `packages/web/src/components/`.

## Fix Focus Areas
- packages/web/src/components/TimeSeriesChart.tsx[12-21]
- packages/web/src/components/TimeSeriesChart.stories.tsx[46-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Nonbinary boolean mislabel ✓ Resolved 🐞 Bug ≡ Correctness
Description
In JournalTrends, boolean series formatValue maps only 1 to “Yes” and maps every other number
to “No”, so out-of-contract values (e.g., 2) would be incorrectly shown as “No” in tooltips.
Because entrySchema accepts any numeric answer_numeric and does not constrain boolean values to
0|1, malformed backend data would not be rejected client-side and would be mislabeled.
Code

packages/web/src/components/JournalPanel.tsx[R401-404]

+        formatValue:
+          question?.data_type === "boolean"
+            ? (value: number) => (value === 1 ? "Yes" : "No")
+            : undefined,
Relevance

●● Moderate

Team often hardens invalid numeric UI cases, but has rejected some malformed-API filtering; boolean
out-of-contract handling uncertain.

PR-#749
PR-#1557

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The boolean formatter introduced in this PR treats any value other than 1 as “No”, while the
client-side schema permits arbitrary numeric answer_numeric values; this combination makes
mislabeling possible if backend data is malformed or changes.

packages/web/src/components/JournalPanel.tsx[91-102]
packages/web/src/components/JournalPanel.tsx[396-405]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Boolean journal observations are formatted with `(value === 1 ? "Yes" : "No")`, which collapses *all* non-`1` values to “No”. If backend data ever contains an out-of-domain number for a boolean question (e.g. `2`), the tooltip will misrepresent it.

### Issue Context
- `entrySchema` parses `answer_numeric` as a generic coerced number (`z.coerce.number().nullable()`) and does not validate boolean domain constraints.
- The new boolean formatter therefore becomes the only place where these values are interpreted.

### Fix Focus Areas
- packages/web/src/components/JournalPanel.tsx[396-404]

### Suggested fix
Update the boolean `formatValue` to explicitly handle `0` and `1`, and provide a safe fallback for unexpected values, e.g.:

```ts
formatValue: question?.data_type === "boolean"
 ? (value: number) => (value === 1 ? "Yes" : value === 0 ? "No" : String(value))
 : undefined,
```

Optionally (stronger), filter boolean series datapoints to `0|1` when building `data` so invalid values don’t render as points at all.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread packages/web/src/components/TimeSeriesChart.tsx
Comment thread packages/web/src/components/JournalPanel.tsx
@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@Asherlc
Asherlc merged commit 805778f into main Jul 29, 2026
101 checks passed
@Asherlc
Asherlc deleted the Asherlc/issue-2156-discrete-behavior-chart branch July 29, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TRACK-03] Binary behavior answers smoothed into impossible fractional lines

2 participants