Skip to content

feat: read a stream's high and low across a quiet range - #1421

Merged
MicBun merged 1 commit into
mainfrom
feat/high-low-quiet-range
Sep 2, 2026
Merged

feat: read a stream's high and low across a quiet range#1421
MicBun merged 1 commit into
mainfrom
feat/high-low-quiet-range

Conversation

@MicBun

@MicBun MicBun commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

resolves: #1419

get_high_value_primitive and get_low_value_primitive selected only rows inside [from, to]. A stream that published nothing during the window returned zero rows, even though the value it published last stood for the whole window and every other read of that stream returns it. Both actions are on the attestation allowlist, and call_dispatch encodes an empty row set rather than raising, so a quiet range produced a signed, fee-charged attestation carrying no datapoint.

Adds the anchor_record pattern from get_record_primitive to both, unioned into the comparison rather than replacing it, so an in-range row still wins when it is genuinely higher or lower. Edited in place in 045 — migrations are not append-only, and 045 has no .prod.sql variant (it touches no bridge namespace, and generate_prod_migrations.py does not list it).

The composed branch was broken too

Writing the "primitive and composed agree" test turned up a second defect. Both composed branches returned nothing for every composed stream, not just quiet ranges:

if $max_value IS NULL OR $row.value > $max_value {

The interpreter does not implement three-valued logic. On the first row $max_value IS NULL is TRUE and $row.value > $max_value is NULL, and TRUE OR NULL evaluates to NULL, which reads as not-taken. The first assignment never happened, so $max_value stayed NULL and the loop discarded every row get_record handed it. Split into IS NULL / ELSEIF in the same commit.

This is the third live instance of that pattern in the repo. maa_do_withdraw (049-maa-funding.sql:117) still carries it; not touched here.

Ties go to the anchor, deliberately

ORDER BY value DESC, event_time ASC means the earliest row wins a tie, and the anchor is always the earliest. So a range whose extreme equals the carried-forward value reports an event_time before from.

That is what the composed branch already does — it walks get_record with a strict > and keeps the first maximum, and get_record leads with its own anchor. The two branches have to agree. It is also the more honest answer once duplicate pruning lands, since the tying in-range row will usually not exist.

Tests

New tests/streams/high_low_value_test.go, six cases:

  • a quiet range returns the carried-forward value from both actions
  • the primitive and composed branches return the same (event_time, value) — the case that caught the second defect
  • the anchor and an in-range row compete: high picks the anchor, low picks the in-range row
  • a tie reports the anchor's event_time
  • a range entirely before the stream's first record still returns nothing
  • a frozen_at that predates the stream still returns nothing

go test -tags kwiltest ./tests/streams/ -run TestHighLowValue passes. kwil-cli utils parse clean.

Note for review

This changes an attestable result. Neither action has ever been attested on mainnet or testnet — mainnet holds 1,011 attestations and testnet 86,966, all of them price_above_threshold, price_below_threshold, value_in_range or index_change_in_range — so no signed result anyone holds moves. The action name is resolved inside block execution, so every validator needs the new binary before the first call that depends on it.

Summary by CodeRabbit

  • Bug Fixes

    • High and low value stream actions now report the value that remained active during a range with no new publications.
    • Corrected high/low value handling for composed streams.
    • Ensured ties use the earliest event time.
    • Prevented values from being returned before a stream’s first record or past its freeze point.
  • Tests

    • Added coverage for quiet ranges, composed streams, anchors, ties, and stream boundaries.

@MicBun MicBun self-assigned this Sep 1, 2026
@holdex

holdex Bot commented Sep 1, 2026

Copy link
Copy Markdown

Time Submission Status

Member # Time Running Total Status Last Update
MicBun 4h ✅ Submitted Sep 1, 2026, 11:06 PM

Submit or update total time with:

@holdex pr submit-time 2h

Add time on top of previous submission with:

@holdex pr add-time 1h30m

See available commands to help comply with our Guidelines.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: ba1a84b4-3487-45cc-99ce-ba0189e31afd

📥 Commits

Reviewing files that changed from the base of the PR and between 325d4a1 and 847fea4.

📒 Files selected for processing (2)
  • internal/migrations/045-high-low-attestation-actions.sql
  • tests/streams/high_low_value_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The high and low value actions now use the last value before the requested range as an anchor. Composed-stream branches also initialize their first result correctly. New schema tests cover quiet ranges, ties, frozen views, and primitive/composed consistency.

Changes

High and low attestation actions

Layer / File(s) Summary
Anchor and composed-branch fixes
internal/migrations/045-high-low-attestation-actions.sql
Primitive high and low actions include the last pre-range event. Composed branches handle null initial values before comparing subsequent rows.
High and low action tests
tests/streams/high_low_value_test.go
Tests cover quiet ranges, primitive and composed results, anchor/range competition, ties, pre-first-record queries, and frozen_at behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 847fe

The PR updates existing high/low reads so quiet ranges and composed streams return the correct datapoint without broadening access. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 clearly describes the main change: reading a stream's high and low values across a quiet range.
Linked Issues check ✅ Passed The migration carries forward the last published value for quiet ranges and fixes composed-stream behavior. The added tests cover quiet ranges, primitive/composed consistency, anchors, ties, and pre-f…
Out of Scope Changes check ✅ Passed The SQL changes, composed-stream fix, and focused tests directly support the linked issue and stated objectives. No unrelated code changes are identified.
Full details: Linked Issues check

Explanation

The migration carries forward the last published value for quiet ranges and fixes composed-stream behavior. The added tests cover quiet ranges, primitive/composed consistency, anchors, ties, and pre-first-record cases required by issue #1419.

Full details: Docstring Coverage

Explanation

Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/high-low-quiet-range

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.

@MicBun

MicBun commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@holdex pr submit-time 4h

@MicBun
MicBun merged commit bd5bb70 into main Sep 2, 2026
8 checks passed
@MicBun
MicBun deleted the feat/high-low-quiet-range branch September 2, 2026 00:23
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.

Problem: traders can't get a high or low from a quiet range

1 participant