Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app/services/heartbeat_import_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def self.import_from_file(file_content, user, on_progress: nil, progress_interva
flush = lambda do
next if heartbeat_batch.empty?
result = HeartbeatIngest.call(user:, mode: :import, heartbeats: heartbeat_batch,
user_agents_by_id:, schedule_rollup_refresh: false)
user_agents_by_id:)
Comment on lines 13 to +14

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.

imported_count += result.persisted_count
errors.concat(result.errors)
heartbeat_batch.clear
Expand All @@ -30,7 +30,6 @@ def self.import_from_file(file_content, user, on_progress: nil, progress_interva

raise StandardError, "Expected a heartbeat export JSON file." if total_count.zero?
flush.call
HeartbeatIngest.schedule_rollup_refresh(user:) if imported_count.positive?

elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time
{ success: true, imported_count:, total_count:,
Expand Down
27 changes: 27 additions & 0 deletions test/services/heartbeat_import_service_test.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,33 @@
require "test_helper"

class HeartbeatImportServiceTest < ActiveSupport::TestCase
include ActiveJob::TestHelper

test "a malformed dump after a committed batch still invalidates rollups" do
original_cache = Rails.cache
original_adapter = ActiveJob::Base.queue_adapter
ActiveJob::Base.queue_adapter = :test
Rails.cache = ActiveSupport::Cache::MemoryStore.new
user = create(:user)
create(:heartbeat, user: user, time: 1_800_000_000.0)
DashboardRollupRefreshService.new(user: user).call
clear_enqueued_jobs
Rails.cache.clear
row = { entity: "old.rb", time: 1_700_000_000.0, type: "file" }.to_json
truncated_dump = '{"heartbeats":[' + ([ row ] * HeartbeatImportService::BATCH_SIZE).join(",") + ',{"entity":'

result = HeartbeatImportService.import_from_file(StringIO.new(truncated_dump), user)

assert_not result[:success]
assert_equal 1, result[:imported_count]
assert_equal 2, user.heartbeats.count
assert DashboardRollup.dirty?(user.id)
assert_enqueued_with(job: DashboardRollupRefreshJob, args: [ user.id ])
ensure
Rails.cache = original_cache
ActiveJob::Base.queue_adapter = original_adapter
end

test "sanitizes null bytes without losing valid batch rows and deduplicates replays" do
user = create(:user)
rows = [
Expand Down
Loading