Skip to content

Add input-reader module#2

Merged
stephenctw merged 1 commit intomainfrom
feature/safe-input-reader
Mar 8, 2026
Merged

Add input-reader module#2
stephenctw merged 1 commit intomainfrom
feature/safe-input-reader

Conversation

@stephenctw
Copy link
Collaborator

@stephenctw stephenctw commented Mar 1, 2026

Add safe-input-reader and related storage logic

@stephenctw stephenctw requested a review from GCdePaula March 1, 2026 12:45
@stephenctw stephenctw self-assigned this Mar 1, 2026
@stephenctw stephenctw force-pushed the feature/safe-input-reader branch 3 times, most recently from 16e155b to 42597c6 Compare March 1, 2026 15:54
Copy link
Collaborator

@GCdePaula GCdePaula left a comment

Choose a reason for hiding this comment

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

Nice work!

I’ve left two inline comments (range split + atomic cursor/input persistence).

There are two follow-ups I suggest we handle on top of my open branch:

  1. Input reader lifecycle
    Right now the reader thread is detached, so we don’t have proper shutdown/join/error propagation to main.
    In my open PR we already have a cleaner lifecycle pattern for background workers; I suggest we wire the reader into that same pattern.

  2. Config surface
    This PR introduces new env knobs.
    My branch has moved config parsing to clap, so we should add the new reader options there to keep configuration consistent.

Given that, my recommendation is:

  • merge my branch first,
  • then rebase this input-reader work on top,
  • and apply the two inline fixes plus the two follow-ups above.

Comment on lines +74 to +76
let blocks = 1 + end_block - start_block;
let half = blocks / 2;
let middle = start_block + half - 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not handling the base case. Moreover, the middle calculation can underflow.

I propose these changes:

if start_block >= end_block {
    // Cannot partition further; bubble original error.
    return Err(vec![e]);
}

let middle = start_block + (end_block - start_block) / 2;

Comment on lines +275 to +277
self.storage.append_safe_direct_inputs(&batch)?;
self.storage
.input_reader_set_last_processed_block(current)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two operations have to be atomic; we need to perform them inside a single db transaction.

I'd create a single function in storage called something like append_safe_inputs_and_advance_cursor(inputs, new_last_processed_block) that performs both operations atomically.

@stephenctw
Copy link
Collaborator Author

Given that, my recommendation is:

  • merge my branch first,
  • then rebase this input-reader work on top,
  • and apply the two inline fixes plus the two follow-ups above.

Sounds good, let's merge your PR first, then adjust mine afterwards.

@stephenctw stephenctw force-pushed the feature/safe-input-reader branch 3 times, most recently from 833bcd4 to 6ae843a Compare March 6, 2026 13:46
@stephenctw stephenctw requested a review from GCdePaula March 6, 2026 13:46
Copy link
Collaborator

@GCdePaula GCdePaula left a comment

Choose a reason for hiding this comment

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

Looks great! I've added one last comment that I should have caught on the previous review.

Copy link
Collaborator

@GCdePaula GCdePaula Mar 7, 2026

Choose a reason for hiding this comment

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

Since this we're at prototyping stages, we don't need to keep compatibility with previous schemas. I think we should fold this change into 0001_schema.sql. No need for ALTER TABLE too, just change the schema of the original table!

@stephenctw stephenctw force-pushed the feature/safe-input-reader branch from 6ae843a to fa02d21 Compare March 7, 2026 15:06
@stephenctw stephenctw requested a review from GCdePaula March 7, 2026 15:07
@stephenctw stephenctw force-pushed the feature/safe-input-reader branch from fa02d21 to 11d5482 Compare March 8, 2026 05:18
@stephenctw stephenctw force-pushed the feature/safe-input-reader branch from 11d5482 to 5fc625b Compare March 8, 2026 05:19
Copy link
Collaborator

@GCdePaula GCdePaula left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

@stephenctw stephenctw merged commit 4b923cd into main Mar 8, 2026
2 checks passed
@stephenctw stephenctw deleted the feature/safe-input-reader branch March 8, 2026 11:00
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.

2 participants