Skip to content

docs(optical): name the file the export actually writes, and map buttons to labels - #863

Merged
ryanbr merged 2 commits into
mainfrom
docs/862-optical-experiment-nits
Jul 27, 2026
Merged

ryanbr merged 2 commits into
mainfrom
docs/862-optical-experiment-nits

Conversation

@ryanbr

@ryanbr ryanbr commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Follow-up to #862, found reviewing it. Docs only — two things that would each stop someone using the harness on their first attempt.

1. The analyze command names a file nothing writes

- swift run whoop-optical-experiment /path/to/noop-whoop5-optical-experiment.jsonl \
+ swift run whoop-optical-experiment /path/to/puffin-deepbuffers.jsonl \

opticalExperimentURL() returns Self.logURL(), which is puffin-deepbuffers.jsonl — the existing capture log, because markers are appended to it rather than written to a file of their own. Copying the documented command fails, and the natural next move is to go hunting for a file that was never created.

Also made the export step say which file it is, since "save the .jsonl file" gives no clue that it is the same log the frames are already going into.

2. The procedure names buttons; the data uses raw values

The capture steps say Off wrist · sensor covered. The JSONL says off_wrist_dark. Those are not the same words, so grepping your own export for what you tapped can quietly return nothing — on a harness whose entire output is that file.

Added the eight-row mapping, and stated that experiment_end is a boundary rather than a phase: it closes the preceding one so later offload records are not attributed past the run, and it never appears as a phase in the report. That behaviour is correct in the analyzer but was not written down anywhere a capturer would look.

Verification

Every mapped value checked against PuffinOpticalExperimentPhase — all eight present, none invented:

enum values : experiment_end, gentle_pressure, off_wrist_dark, off_wrist_room_light,
              on_wrist_again, on_wrist_still, paced_breathing_normal, paced_breathing_slow
doc ⊆ enum  : True

Both gates green. No code touched.

Worth saying why this is worth a PR rather than a shrug: #862 is a harness with no value until someone takes a labelled capture, and both of these defects land squarely on the person trying to take one.

ryanbr added 2 commits July 26, 2026 21:01
…ons to labels

Two things that would each stop someone using the #862 harness on their first try.

The analyze command told you to run the CLI against
`noop-whoop5-optical-experiment.jsonl`. Nothing writes that. `opticalExperimentURL()`
returns `Self.logURL()`, which is `puffin-deepbuffers.jsonl` — the existing capture log,
because markers are appended to it rather than written separately. Copying the documented
command fails, and the natural next move is to hunt for a file that does not exist.

The procedure names buttons ("Off wrist · sensor covered") while the JSONL and the report
carry raw values (`off_wrist_dark`). Those are not always the same words, so grepping an
export for what you tapped can quietly return nothing. Added the full eight-row mapping,
and said plainly that `experiment_end` is a boundary rather than a phase — it closes the
preceding one and never appears as a phase in the report.

Verified every mapped value against PuffinOpticalExperimentPhase: all eight present, none
invented.

Docs only. The harness is worth nothing until someone runs it, and both of these land on
the person trying to.
…checked one side

I claimed the analyze command named a file nothing writes. It does write it:
SettingsView exports through FileExport.timestampedName("noop-whoop5-optical-experiment"),
so the file the user actually saves IS noop-whoop5-optical-experiment-<timestamp>.jsonl.
The original doc described the exported file; I checked only the on-device writer
(logURL -> puffin-deepbuffers.jsonl) and concluded the doc was wrong.

Pointing the command at puffin-deepbuffers.jsonl made it worse: that name exists only
inside the app container, so a reader following step 9 would have saved one file and been
told to analyse another.

Both names are now documented, with the reason they differ — the markers are appended to
the existing capture log, and the export renames on the way out. The analyzer only reads
contents, so either file works.

The label-mapping table stands: verified against PuffinOpticalExperimentPhase for the raw
values and against the String(localized:) cases for the button names, so both columns come
from code rather than from the prose I was correcting.
@ryanbr

ryanbr commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Reviewed my own PR and had to reverse half of it.

I claimed the analyze command named a file nothing writes. It does. SettingsView exports via FileExport.timestampedName("noop-whoop5-optical-experiment"), so the file a user actually saves is noop-whoop5-optical-experiment-<timestamp>.jsonl. The original doc was describing the exported file. I checked only the on-device writer — logURL() → puffin-deepbuffers.jsonl — and concluded the doc was wrong.

Pointing the command at puffin-deepbuffers.jsonl made it worse: that name exists only inside the app container, so a reader following step 9 would have saved one file and been told to analyse a different one. I turned a non-bug into a real one.

Both names are now documented together with the reason they differ — markers are appended to the existing capture log, and the export renames on the way out. The analyzer reads contents, so either file works.

The label-mapping table stands, and I re-verified it properly this time: raw values against PuffinOpticalExperimentPhase, button names against the String(localized:) cases. Both columns come from code rather than from the prose I was correcting — which is exactly the check I skipped on the filename.

@ryanbr
ryanbr merged commit 5be4b52 into main Jul 27, 2026
2 checks passed
@ryanbr
ryanbr deleted the docs/862-optical-experiment-nits branch July 27, 2026 04:08
DX23876 added a commit to DX23876/noop that referenced this pull request Aug 1, 2026
Classic Today kept an older, larger OverviewHRChart with its own pinch-zoom
(ryanbr#829), a hint row and sleep / workout / Charge / Effort markers, plus a live
bpm badge that was not tappable. Liquid draws a compact live-HR card and hangs
the whole-day reading on a "Full day" link into the Deep Timeline. Classic now
hosts the SAME card.

LiquidLiveHR moves out of LiquidTodayView into its own file — it was already
styled purely in StrandPalette / StrandFont, so this is a move, not a restyle.
Two optional parameters carry the classic call site, both defaulted so Liquid
is unchanged: `headerRoute` makes the read-out row (not the scrubbable thread,
which would swallow the drag as a tap) a link, so a tap on the live bpm opens
the Deep Timeline; `showsLive` is false on a past day, where a live number says
nothing about the day on screen.

Removed with the chart: the ryanbr#829 zoom window, its hint and reset, the pure
reclampHrZoom rule and its tests, the HRChartFrameKey mask, the cached hrAxis,
and the four marker properties — all of that reading lives in the Deep Timeline,
and OverviewHRChart itself is untouched and still drawn there. The day-swipe now
yields to the thread via hrScrubbing / hrScrubEndedAt, as Liquid already does.
The ryanbr#863 calibrating / empty-day branch is deliberately unchanged.

Android's HeartRateTrendCard is not changed: Android has no Liquid Today, so
there is no divergence to close. No analytics and no stored-data change.

Verified: xcodegen generate, then Strand (macOS) and NOOPiOS (iOS) both build
(no default CI compiles app-target Swift); StrandTests green; StrandDesign green
apart from a palette test that fails identically on a clean tree. Not exercised
on a strap or in the simulator.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…ame exact (ryanbr#863)

Follow-up to ryanbr#862. Three fixes to the capture doc, all of which land on the person trying to
run the harness — which is the only way ryanbr#862 produces any value at all.

The procedure names buttons ("Off wrist - sensor covered") while the JSONL and the report carry
raw values (off_wrist_dark). Those are not always the same words, so grepping your own export for
what you tapped can quietly return nothing. Added the full eight-row mapping. Both columns were
verified against code rather than transcribed from the prose being corrected: raw values against
PuffinOpticalExperimentPhase, button names against the String(localized:) cases.

Wrote down that experiment_end is a boundary rather than a phase — it closes the preceding one so
later offload records are not attributed past the run, and never appears as a phase in the report.
Correct in the analyzer, documented nowhere a capturer would look.

Made the export filename exact. FileExport.timestampedName always appends -yyMMdd-HHmm, so
noop-whoop5-optical-experiment.jsonl never exists; it is
noop-whoop5-optical-experiment-<timestamp>.jsonl. Step 9 now also names the on-device file
(puffin-deepbuffers.jsonl) and says why they differ: markers are appended to the existing capture
log, and the export renames on the way out. The analyzer reads contents, so either file works.

An earlier revision of this PR pointed the analyze command at puffin-deepbuffers.jsonl on the
grounds that nothing wrote the other name. That was wrong and was reverted before merge — the
export does write it. I had checked only the on-device writer and treated the disagreement as
proof the doc was wrong, when both names were real and described different things.

Docs only.
ryanbr added a commit that referenced this pull request Oct 9, 2026
The Today HR card's empty state opened "Calibrating" and asserted "no heart
rate banked yet today", then offered "your curve fills in as the strap
offloads". None of that is established on that branch. The only thing the
card knows is that the active-strap-plus-imports union came back with fewer
than two 5-minute buckets for the window. It does not know whether a strap is
calibrating, whether it has offloaded, or whether rows are banked under a
source the union cannot reach, and naming an unchecked cause sends a reader
looking in the wrong place.

The same line also fired on a day holding exactly one bucket, where "no heart
rate" is simply false.

So the state now splits in two and describes the stored rows:

  0 buckets  No heart rate stored for today yet. The curve draws once the
             strap offloads, or once an import brings one in.
  1 bucket   Only one five-minute block of heart rate is stored for today.
             The curve needs two of them to draw a line.

Both platforms, all nine locales. The split reads the DAY's bucket count
rather than the windowed subset, because both lines speak about today: a
narrow rolling window that happens to exclude the one stored block must not
make the card claim the day holds nothing.

Android's selection is extracted into a pure hrEmptyMessageRes so it can be
pinned without a Compose host; Apple's is hoisted into hrEmptyTitle and
hrEmptyDetail rather than nested as ternaries, since TodayView already sits
near the type-check budget. HrEmptyMessageTest covers all six selections and
carries source tripwires for the Apple twin, the retired resource key and the
call site, none of which a JVM test can otherwise reach.

Refs #863
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.

1 participant