correlate journal fields that carry a number, not just a tag - #41
Conversation
A tag can only answer "were the tagged days different?". A field that carries a dose — three coffees, 700 ml, mood 4 out of 5 — carries more than that, and collapsing it to present/absent throws away the part people actually want to know: whether one coffee is different from five. This is Spearman's rho rather than Pearson. Self-reported dose is ordinal at best and routinely spiky, and ranks are invariant to both; a monotone relationship is all these fields can support. Alongside it, a Theil-Sen slope in the outcome's own units, because "about 4 ms of RMSSD per extra coffee" is the sentence worth reading and a correlation coefficient is not. It refuses to answer far more often than the tag path does. Eight paired days minimum, a floor on the magnitude, and a Fisher-z interval that has to exclude zero — over five days a rank correlation of 1 is routine noise, and that is exactly when it looks most convincing. A field that was never filled in is excluded pairwise rather than read as a zero: not logging caffeine is not the same as logging none, and treating it as none invents a point at the bottom of the dose range, where a correlation is most sensitive. Perfect correlations are handled rather than dropped, since a monotone field saturates rho immediately. The saturated value is pulled in by 1/(2n), so the interval still excludes zero but widens as the sample shrinks.
📝 WalkthroughWalkthroughChangesNumeric journal correlation support adds public data models, Spearman correlation, statistical effects, confidence intervals, meaningfulness status, missing-data handling, alignment validation, deterministic sorting, and tests. Numeric journal correlations
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant journalNumericCorrelations
participant JournalNumericDay
participant Outcomes
Caller->>journalNumericCorrelations: provide journal, dates, outcomes, and thresholds
journalNumericCorrelations->>JournalNumericDay: collect aligned numeric values
journalNumericCorrelations->>Outcomes: read outcome series
journalNumericCorrelations->>journalNumericCorrelations: calculate rho, confidence interval, slope, and meaningfulness
journalNumericCorrelations-->>Caller: return sorted JournalNumericCorrelation results
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/onehz/human/coaching.dart`:
- Around line 660-672: Update the Fisher z interval calculation in the
rhoLow/rhoHigh logic to use the Spearman standard error sqrt((1.0 + r * r / 2.0)
/ (n - 3)) instead of 1.0 / sqrt(n - 3), using the already clamped correlation
value r. Keep the existing confidence bounds and transformation unchanged.
In `@test/onehz/journal_numeric_correlations_test.dart`:
- Around line 264-273: Add tests for the public minN and minAbsRho parameters in
journalNumericCorrelations: use minN between 4 and 7 to verify rho is computed
while rhoLow and rhoHigh still follow the n > 3 rule, and use minAbsRho above a
computed absolute rho to verify meaningful is false while insufficient remains
false.
- Around line 12-23: Update the _dates helper to generate valid calendar dates
by rolling days beyond each month’s length into subsequent months, including the
values needed by run(40), instead of emitting day numbers above 31. In _days,
compute _dates(values.length) once before the comprehension and reuse that list
for every JournalNumericDay.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3927fd09-9691-4582-a4de-19a551435795
📒 Files selected for processing (2)
lib/src/onehz/human/coaching.darttest/onehz/journal_numeric_correlations_test.dart
|
The standard error was a real error, and it pointed the wrong way. Fisher's 1/sqrt(n−3) is derived for Pearson's r under bivariate normality; ranks are neither, and it runs narrow for rho. Since the whole verdict hangs on that interval excluding zero, too narrow means weaker relationships get through — the opposite of what this function is for. It uses the Bonett & Wright (2000) rank standard error now. The test dates were producing 2026-01-32 and onwards. Harmless while a date is an opaque key, and a baffling failure the day that stops being true, so they roll over properly now. The per-element list rebuild is gone too. Added the two gate cases. They turned out to be worth having for a reason beyond coverage: separating the strength floor from the interval makes it explicit that "too weak to mention" and "not enough evidence" are different answers, and the caller may well want to phrase them differently. The second test also pins that four pairs still gets an interval — it is just so wide it cannot clear zero, which is the right answer — while three gets none at all. |
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/onehz/journal_numeric_correlations_test.dart`:
- Around line 347-365: Correct the boundary test around the existing correlation
effects setup: ensure the four-pair case uses exactly four observations, then
add an n == 4 case with minN: 4 that asserts both confidence bounds are present
and the interval includes zero. Keep the existing n == 3 no-interval assertions
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6b24630-557d-4c8c-ae0e-445fe411a68e
📒 Files selected for processing (2)
lib/src/onehz/human/coaching.darttest/onehz/journal_numeric_correlations_test.dart
| // Four pairs is where the interval gets absurdly wide but still exists | ||
| // — and being unable to exclude zero is exactly the right answer there. | ||
| expect(e.rhoLow!, lessThan(0), reason: 'five days cannot clear zero'); | ||
| expect(e.meaningful, isFalse); | ||
|
|
||
| // At three the standard error is undefined outright, so there is no | ||
| // interval at all and therefore no verdict. | ||
| final three = _dates(3); | ||
| final e3 = journalNumericCorrelations( | ||
| journal: [ | ||
| for (var i = 0; i < 3; i++) | ||
| JournalNumericDay(three[i], {'water': i.toDouble()}), | ||
| ], | ||
| dates: three, | ||
| outcomes: {'readiness': [for (var i = 0; i < 3; i++) 50.0 + i]}, | ||
| minN: 3, | ||
| ).single.effects.single; | ||
| expect(e3.rho, closeTo(1.0, 1e-9)); | ||
| expect(e3.rhoLow, isNull, reason: 'n > 3 is required for the SE'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Test the four-pair confidence-interval boundary.
The comment at Line 347 describes four pairs, but e uses five pairs. The suite can pass if the production condition changes from n > 3 to n > 4. Add an n == 4, minN: 4 case. Assert that both bounds exist and that the interval includes zero.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/onehz/journal_numeric_correlations_test.dart` around lines 347 - 365,
Correct the boundary test around the existing correlation effects setup: ensure
the four-pair case uses exactly four observations, then add an n == 4 case with
minN: 4 that asserts both confidence bounds are present and the interval
includes zero. Keep the existing n == 3 no-interval assertions unchanged.
1 participant
Groundwork for typed journal entries in the app: mood, water, caffeine dose, sleep-quality ratings. The existing
journalCorrelationstakes a tag set, so the only question it can ask is "were the tagged days different?" — a field carrying a dose needs a statistic that can tell one coffee from five.Spearman's rho rather than Pearson: self-reported dose is ordinal at best and routinely spiky (one six-coffee day), and ranks are invariant to both. A monotone relationship is what "more of this goes with worse recovery" actually claims, and it is all these fields can support. Ties share their mean rank, because journal fields are full of ties — mood is 1–5 and most people log the same two coffees most days, and ranking those arbitrarily would invent an ordering nobody reported.
Reported alongside rho is a Theil–Sen slope in the outcome's own units. "About 4 ms of RMSSD per extra coffee" is the sentence worth surfacing; the coefficient carries whether the relationship holds at all, the slope carries what it means.
The gate is deliberately stricter than the tag path's. Eight paired days minimum, a floor on |rho|, and a Fisher-z interval that has to exclude zero. Over five days a rank correlation of 1 turns up constantly, and that is precisely when it reads as certainty.
Two decisions worth calling out:
Summary by CodeRabbit
New Features
Tests