Skip to content

PE-8687: show Turbo-uploaded files immediately with pending status#2089

Merged
vilenarios merged 3 commits intodevfrom
PE-8687-show-uploaded-files-immediately
Nov 25, 2025
Merged

PE-8687: show Turbo-uploaded files immediately with pending status#2089
vilenarios merged 3 commits intodevfrom
PE-8687-show-uploaded-files-immediately

Conversation

@vilenarios
Copy link
Copy Markdown
Collaborator

@vilenarios vilenarios commented Oct 29, 2025

Remove blocking break statement in sync process that prevented unmined
transactions from being displayed. Files uploaded via Turbo are now
visible immediately with pending/orange status indicators.

Changes:

  • Process unmined transactions (block == null) with block height 0
  • Group unmined transactions separately in blockHistory
  • Leverage existing transaction status system for pending→confirmed flow
  • Existing cleanup marks transactions as failed if unmined > 24 hours

Impact:

  • Turbo-uploaded files appear in 2-5 seconds instead of 30+ minutes
  • Better UX for cross-platform uploads (ardrive-core-js → web)
  • No breaking changes, uses existing pending transaction infrastructure

Summary by CodeRabbit

  • Bug Fixes
    • Transaction history now preserves continuity by grouping unmined transactions into a virtual block, preventing premature termination and providing more complete, continuous records.
  • New Features
    • History processing is now block-height aware, improving how recent/unmined transactions are attributed when presenting entity history.

✏️ Tip: You can customize this high-level summary in your review settings.

  Remove blocking break statement in sync process that prevented unmined
  transactions from being displayed. Files uploaded via Turbo are now
  visible immediately with pending/orange status indicators.

  Changes:
  - Process unmined transactions (block == null) with block height 0
  - Group unmined transactions separately in blockHistory
  - Leverage existing transaction status system for pending→confirmed flow
  - Existing cleanup marks transactions as failed if unmined > 24 hours

  Impact:
  - Turbo-uploaded files appear in 2-5 seconds instead of 30+ minutes
  - Better UX for cross-platform uploads (ardrive-core-js → web)
  - No breaking changes, uses existing pending transaction infrastructure
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 29, 2025

Walkthrough

Added an optional currentBlockHeight parameter and changed history construction to compute blockHeight = transaction.block?.height ?? currentBlockHeight ?? lastBlockHeight; unmined transactions are no longer causing an early break but are aggregated under the computed blockHeight; call site updated to forward currentBlockHeight.

Changes

Cohort / File(s) Summary
Arweave history construction
lib/services/arweave/arweave_service.dart
Added int? currentBlockHeight param; compute blockHeight = transaction.block?.height ?? currentBlockHeight ?? lastBlockHeight; use computed blockHeight to decide when to start new BlockEntities; removed early exit for unmined transactions and removed redundant augmentation branch.
Sync callsite
lib/sync/domain/repositories/sync_repository.dart
Forward currentBlockHeight when invoking createDriveEntityHistoryFromTransactions(...).
Project metadata
pubspec.yaml
(unchanged in content summary)

Sequence Diagram(s)

sequenceDiagram
    participant Sync as SyncRepository
    participant Arw as ArweaveService
    participant Agg as BlockAggregator

    Note over Sync,Arw: caller may supply currentBlockHeight (optional)
    Sync->>Arw: createDriveEntityHistoryFromTransactions(entityTxs, ..., currentBlockHeight?)
    loop per transaction
        Arw->>Arw: compute blockHeight = transaction.block?.height ?? currentBlockHeight ?? lastBlockHeight
        alt blockHeight changed from previous
            Arw->>Agg: emit previous BlockEntities
        end
        Arw->>Agg: add transaction to BlockEntities(blockHeight)
    end
    Arw->>Agg: emit final BlockEntities
    Agg-->>Sync: DriveEntityHistory (grouped by computed blockHeight)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify blockHeight precedence and null-safety (transaction.block, currentBlockHeight, lastBlockHeight).
  • Confirm ordering and boundaries when switching block groups (especially around virtual/currentBlockHeight).
  • Check call sites for correct propagation of currentBlockHeight and tests for unmined transactions.

Poem

🐰 I hop through blocks both real and guessed,
Unmined tales tucked safe inside my nest.
Heights whispered in, no sudden stop or sigh,
History stitched neat beneath the sky,
🥕 A rabbit hums — the ledger marches by.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the primary change: showing Turbo-uploaded files immediately with pending status, which aligns with the main objective of removing the blocking break for unmined transactions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch PE-8687-show-uploaded-files-immediately

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 885b4cd and 23ba8cf.

📒 Files selected for processing (2)
  • lib/services/arweave/arweave_service.dart (2 hunks)
  • lib/sync/domain/repositories/sync_repository.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/services/arweave/arweave_service.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: pre-build / test-and-lint
🔇 Additional comments (1)
lib/sync/domain/repositories/sync_repository.dart (1)

1101-1101: LGTM! Parameter forwarding enables pending transaction display.

The addition of currentBlockHeight parameter correctly forwards the block height information to the arweave service layer, enabling unmined transactions to be processed and displayed with pending status as described in the PR objectives.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/services/arweave/arweave_service.dart (1)

436-439: Critical: Incorrect lastBlockHeight when unmined transactions are processed last.

If unmined transactions are processed last (or exist anywhere in the list), blockHistory.last.blockHeight could be 0, which would be returned as the lastBlockHeight in the result. This breaks subsequent syncs because they would think the last processed block was 0, potentially causing reprocessing of the entire history.

The returned lastBlockHeight should be the highest mined block height, excluding virtual block 0.

Apply this diff to track the maximum mined block height correctly:

   final blockHistory = <BlockEntities>[];
+  int maxMinedBlockHeight = lastBlockHeight;
 
   for (var i = 0; i < entityTxs.length; i++) {
     final transaction = entityTxs[i].transactionCommonMixin;
 
     final tags = HashMap.fromIterable(
       transaction.tags,
       key: (tag) => tag.name,
       value: (tag) => tag.value,
     );
 
     if (driveKey != null && tags[EntityTag.cipherIv] == null) {
       logger.d('skipping unnecessary request for a broken entity');
       continue;
     }
 
     // Process unmined transactions (block == null) with block height 0
     // They will be marked as pending and cleaned up by _updateTransactionStatuses
     // if they remain unmined beyond the timeout thresholds (8-24 hours)
     final blockHeight = transaction.block?.height ?? 0;
+    
+    // Track the highest mined block height for the next sync
+    if (blockHeight > 0 && blockHeight > maxMinedBlockHeight) {
+      maxMinedBlockHeight = blockHeight;
+    }
 
     if (blockHistory.isEmpty ||
         blockHistory.last.blockHeight != blockHeight) {
       blockHistory.add(BlockEntities(blockHeight));
     }

Then update the return statement:

   return DriveEntityHistory(
-    blockHistory.isNotEmpty ? blockHistory.last.blockHeight : lastBlockHeight,
+    maxMinedBlockHeight,
     blockHistory,
   );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc4f482 and 80eecd9.

📒 Files selected for processing (1)
  • lib/services/arweave/arweave_service.dart (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: pre-build / test-and-lint
🔇 Additional comments (2)
lib/services/arweave/arweave_service.dart (2)

368-371: LGTM!

The block grouping logic correctly creates new BlockEntities containers when processing a new block height, including the virtual block 0 for unmined transactions.


363-366: Cleanup mechanism verified—no issues found.

The referenced _updateTransactionStatuses exists in lib/sync/domain/repositories/sync_repository.dart and correctly handles unmined transactions. Pending transactions are marked as failed when either (1) unconfirmed for 8+ hours (line 715), or (2) creation date exceeds pendingWaitTime of 1 day (line 777). The blockHeight normalization to 0 is sound.

Comment thread lib/services/arweave/arweave_service.dart Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 29, 2025

Visit the preview URL for this PR (updated for commit 23ba8cf):

https://ardrive-web--pr2089-pe-8687-show-uploade-37af6zrf.web.app

(expires Fri, 28 Nov 2025 19:06:37 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0

Removed duplicate block history check at lines 404-408 that was redundant
with the check at lines 368-371. Between these two checks, nothing mutates
blockHistory or blockHeight, making the second check unreachable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80eecd9 and 885b4cd.

📒 Files selected for processing (1)
  • lib/services/arweave/arweave_service.dart (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: pre-build / test-and-lint
🔇 Additional comments (1)
lib/services/arweave/arweave_service.dart (1)

363-366: Verify the actual timeout duration and transaction ordering.

The comment references a 8-24 hour timeout for cleaning up unmined transactions, but the code shows cleanup uses pendingWaitTime (undefined in search results) with a 45-minute threshold. The actual timeout duration needs clarification.

Additionally, verify that createDriveEntityHistoryFromTransactions receives transactions in the expected order. The method constructs blockHistory by iterating through transactions and returns blockHistory.last.blockHeight as the new sync checkpoint. If transactions arrive in descending block order (typical for GraphQL), unmined transactions (blockHeight=0) may end up last in the history, causing the method to return 0 instead of the actual highest block—corrupting sync state for the next run.

Comment thread lib/services/arweave/arweave_service.dart
Replace block height 0 assignment for unmined transactions with
currentBlockHeight to prevent lastBlockHeight contamination that would
cause full re-sync on subsequent syncs.

Changes:
- Add currentBlockHeight parameter to createDriveEntityHistoryFromTransactions
- Use currentBlockHeight ?? lastBlockHeight fallback for unmined transactions
- Pending files now appear at current block and update when mined
- Prevents sync cursor from being reset to 0

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@arielmelendez
Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 21, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@vilenarios vilenarios merged commit 46d3917 into dev Nov 25, 2025
8 checks passed
@vilenarios vilenarios deleted the PE-8687-show-uploaded-files-immediately branch November 25, 2025 00:38
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