Skip to content

Invalidate dashboard rollups after each committed import batch - #1690

Open
skyfallwastaken wants to merge 2 commits into
fix-import-null-bytesfrom
fix-partial-import-rollups
Open

Invalidate dashboard rollups after each committed import batch#1690
skyfallwastaken wants to merge 2 commits into
fix-import-null-bytesfrom
fix-partial-import-rollups

Conversation

@skyfallwastaken

Copy link
Copy Markdown
Member

Summary of the problem

An import could commit an earlier batch then fail parsing a later batch without scheduling a dashboard refresh.

Describe your changes

Let every persisted batch invalidate rollups and use the existing queue coalescing. Partial failures remain reported as failures while committed history is reflected by subsequent refreshes.

Screenshots / Media

Not applicable: backend-only changes.

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR moves dashboard rollup invalidation from import completion to each persisted heartbeat batch, ensuring that committed data triggers refresh work even if a later batch fails to parse.

  • Restores the default HeartbeatIngest refresh scheduling for every import batch.
  • Removes the single refresh request previously made after successful parsing.
  • Adds a regression test covering a malformed dump after an earlier batch commits.

Confidence Score: 4/5

The PR is not yet safe to merge because a long-running import can still leave dashboard rollups stale after the queued refresh overlaps later batches.

The previous refresh-race finding remains outstanding: each batch requests refresh work, but queue coalescing can prevent later batches from scheduling a follow-up while the first delayed job is running, allowing that job to clear the shared dirty marker after refreshing from an incomplete import.

Files Needing Attention: app/services/heartbeat_import_service.rb

Important Files Changed

Filename Overview
app/services/heartbeat_import_service.rb Enables rollup invalidation after every persisted import batch, but the previously reported refresh-versus-import race remains outstanding.
test/services/heartbeat_import_service_test.rb Adds durable-state and queue assertions for a malformed import that fails after committing its first batch.

Sequence Diagram

sequenceDiagram
    participant Import as HeartbeatImportService
    participant Ingest as HeartbeatIngest
    participant DB as Heartbeat database
    participant Queue as Refresh queue
    participant Refresh as Rollup refresh

    loop Each parsed batch
        Import->>Ingest: Persist heartbeat batch
        Ingest->>DB: Commit accepted heartbeats
        Ingest->>Queue: Mark dirty and coalesce refresh
    end
    Queue->>Refresh: Run delayed refresh
    Refresh->>DB: Read current heartbeat set
Loading

Reviews (2): Last reviewed commit: "Isolate the import scheduling regression..." | Re-trigger Greptile

Comment on lines 13 to +14
result = HeartbeatIngest.call(user:, mode: :import, heartbeats: heartbeat_batch,
user_agents_by_id:, schedule_rollup_refresh: false)
user_agents_by_id:)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Refresh can miss later batches

If an import lasts beyond the refresh job's two-minute delay, the first batch's job can start while later batches are still being imported. Those later batches mark the rollup dirty, but the existing enqueue reservation prevents them from scheduling another job. The running refresh can then clear the shared dirty marker after building from an incomplete heartbeat set, leaving stale dashboard rollups with no follow-up refresh queued.

Knowledge Base Used:

Prompt To Fix With AI
This is a comment left during a code review.
Path: app/services/heartbeat_import_service.rb
Line: 13-14

Comment:
**Refresh can miss later batches**

If an import lasts beyond the refresh job's two-minute delay, the first batch's job can start while later batches are still being imported. Those later batches mark the rollup dirty, but the existing enqueue reservation prevents them from scheduling another job. The running refresh can then clear the shared dirty marker after building from an incomplete heartbeat set, leaving stale dashboard rollups with no follow-up refresh queued.

**Knowledge Base Used:**
- [Heartbeat tracking and ingestion](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/docs/heartbeat-tracking.md)
- [Dashboard rollups and caching](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/docs/dashboard-rollups-and-caching.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

skyfallwastaken and others added 2 commits September 6, 2026 14:16
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: Amp <amp@ampcode.com>
@skyfallwastaken
skyfallwastaken force-pushed the fix-partial-import-rollups branch from 7e8ef0e to 617afea Compare September 6, 2026 13:16
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