Centralize heartbeat ingestion logic

[skyfallwastaken]

Heartbeat code was messy and duplicated. No more!

[greptile-apps]

Greptile Summary

This PR extracts the heartbeat ingestion logic that was previously split across HackatimeController and HeartbeatImportService into a single HeartbeatIngest service, parameterised by mode: :direct or mode: :import. A new test file provides good coverage of the happy paths.

• New HeartbeatIngest service centralizes normalization, deduplication, persistence, rollup scheduling, and project-mapping queueing, with per-heartbeat error isolation and typed Result/Item value objects returned to callers.
• HeartbeatImportService now delegates batches of raw heartbeats directly to HeartbeatIngest, removing its own flush_batch, normalize_imported_heartbeat, and parse_user_agent helpers.
• HeartbeatImportRunner.complete_run! no longer schedules DashboardRollupRefreshJob; that responsibility moved into HeartbeatImportService, conditioned on imported_count.positive?.

Confidence Score: 4/5

The refactoring is structurally sound with no new breaking changes on the core heartbeat path, but three subtle behavioral shifts around rollup scheduling, last-language tracking on persist failure, and batch-size measurement are worth resolving before merge.

The rollup-scheduling gap (skipped when every heartbeat in an import is a duplicate) is the most user-visible change in behavior. The last-language leak on persist failure and the raw-vs-deduplicated BATCH_SIZE shift are edge cases unlikely to surface in normal usage but diverge from the previous implementation's intent.

app/services/heartbeat_import_runner.rb (rollup scheduling removal), app/services/heartbeat_ingest.rb (last_language update ordering), app/services/heartbeat_import_service.rb (BATCH_SIZE comparison change)

Important Files Changed

Filename	Overview
app/services/heartbeat_ingest.rb	New centralized service handling both :direct and :import modes; logic is well-extracted with good per-heartbeat error isolation, Result/Item value objects, and rollup scheduling per insert.
app/controllers/api/hackatime/v1/hackatime_controller.rb	handle_heartbeat is cleanly delegated to HeartbeatIngest; no logic left in the controller beyond mapping Result items to HTTP tuples.
app/services/heartbeat_import_service.rb	Batching loop now passes raw heartbeats to HeartbeatIngest; deduplication is delegated, but BATCH_SIZE is now counted against undeduplicated raw heartbeats instead of the post-dedup seen_hashes count as before.
app/services/heartbeat_import_runner.rb	DashboardRollupRefreshJob.schedule_for removed from complete_run!; rollup is now owned by HeartbeatIngest/HeartbeatImportService, which skips scheduling when imported_count is zero.
test/services/heartbeat_ingest_test.rb	Good coverage of happy paths (direct persist, dedup, LAST_LANGUAGE resolution, import dedup + rollup); missing tests for error/failure paths and user_agents_by_id resolution in import mode.

Sequence Diagram

sequenceDiagram
    participant Controller as HackatimeController
    participant ImportService as HeartbeatImportService
    participant ImportRunner as HeartbeatImportRunner
    participant Ingest as HeartbeatIngest
    participant DB as Heartbeat (DB)
    participant Job as DashboardRollupRefreshJob

    Note over Controller: Direct (live) path
    Controller->>Ingest: call(mode: :direct, heartbeats:, request_context:)
    loop each heartbeat
        Ingest->>Ingest: normalize_direct_heartbeat
        Ingest->>DB: find_by(fields_hash)
        alt new heartbeat
            Ingest->>DB: insert(returning: ...)
            Ingest->>Job: schedule_rollup_refresh
        end
        Ingest->>Ingest: queue_project_mapping
    end
    Ingest-->>Controller: Result(items)

    Note over ImportRunner: Import path
    ImportRunner->>ImportService: import_from_file(file, user)
    loop each BATCH_SIZE raw heartbeats
        ImportService->>Ingest: call(mode: :import, schedule_rollup_refresh: false)
        Ingest->>DB: upsert_all(seen_hashes.values)
        Ingest-->>ImportService: Result(persisted_count, errors)
    end
    ImportService->>Job: schedule_rollup_refresh (if imported_count > 0)
    ImportService-->>ImportRunner: result hash
    ImportRunner->>ImportRunner: complete_run!

Loading

Comments Outside Diff (1)

1. app/services/heartbeat_import_runner.rb, line 280-294 (link)

   Rollup refresh silently skipped for all-duplicate imports

   DashboardRollupRefreshJob was removed from complete_run! and is now scheduled inside HeartbeatImportService.import_from_file only when imported_count.positive?. If every heartbeat in an import batch is a duplicate, imported_count stays at 0, no rollup is enqueued, and complete_run! exits without scheduling one. Before this PR, the job was always scheduled at the end of a successful run regardless of the duplicate ratio.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: app/services/heartbeat_import_runner.rb
   Line: 280-294
   
   Comment:
   **Rollup refresh silently skipped for all-duplicate imports**
   
   `DashboardRollupRefreshJob` was removed from `complete_run!` and is now scheduled inside `HeartbeatImportService.import_from_file` only when `imported_count.positive?`. If every heartbeat in an import batch is a duplicate, `imported_count` stays at 0, no rollup is enqueued, and `complete_run!` exits without scheduling one. Before this PR, the job was always scheduled at the end of a successful run regardless of the duplicate ratio.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
app/services/heartbeat_import_runner.rb:280-294
**Rollup refresh silently skipped for all-duplicate imports**

`DashboardRollupRefreshJob` was removed from `complete_run!` and is now scheduled inside `HeartbeatImportService.import_from_file` only when `imported_count.positive?`. If every heartbeat in an import batch is a duplicate, `imported_count` stays at 0, no rollup is enqueued, and `complete_run!` exits without scheduling one. Before this PR, the job was always scheduled at the end of a successful run regardless of the duplicate ratio.

### Issue 2 of 4
app/services/heartbeat_ingest.rb:48-56
**`last_language` updated before persistence; leaks into subsequent heartbeats on persist failure**

`last_language` is updated from `attrs[:language]` on line 46, before `persist_direct_heartbeat` is called. If `persist_direct_heartbeat` raises (e.g. a DB error), the rescue branch is taken, but `last_language` has already been mutated to reflect the failed heartbeat's language. Every subsequent heartbeat in the same batch will resolve `<<LAST_LANGUAGE>>` to the language of a heartbeat that was never actually stored.

### Issue 3 of 4
app/services/heartbeat_import_service.rb:24-32
**`BATCH_SIZE` now measured against raw, un-deduplicated heartbeats**

Previously `seen_hashes.size >= BATCH_SIZE` compared against the number of deduplicated heartbeats (duplicate entries collapsed into the same hash key). Now `heartbeat_batch.size >= BATCH_SIZE` counts raw, potentially duplicate entries. A batch with many duplicates can grow substantially larger than `BATCH_SIZE` distinct records before being flushed, which could affect memory pressure during large imports.

### Issue 4 of 4
test/services/heartbeat_ingest_test.rb:1-10
**Missing tests for error paths and `user_agents_by_id` resolution**

The test suite covers happy paths and deduplication well, but two scenarios are unexercised: (1) a malformed/invalid heartbeat in `:direct` mode to verify the per-heartbeat rescue produces a `:failed` item and doesn't abort the rest of the batch; (2) the `:import` mode path when `user_agents_by_id` is populated, to confirm editor/OS resolution from the agent map works after the extraction into `HeartbeatIngest`.

_{Reviews (1): Last reviewed commit: "Centralize heartbeat ingestion logic" | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR centralizes heartbeat normalization, deduplication, and rollup-refresh behavior into a new HeartbeatIngest service so both direct API ingestion and import ingestion share one code path.

Changes:

• Adds HeartbeatIngest to handle direct and import heartbeat processing.
• Refactors the Hackatime API controller and import service to use the new shared service.
• Updates import completion flow and adds service-level tests for ingestion behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
app/services/heartbeat_ingest.rb	New shared service for direct/import heartbeat normalization, persistence, deduplication, and follow-up jobs.
app/services/heartbeat_import_service.rb	Replaces inline import normalization/upsert logic with batched calls into HeartbeatIngest.
app/services/heartbeat_import_runner.rb	Removes runner-level rollup scheduling now that import scheduling is delegated elsewhere.
app/controllers/api/hackatime/v1/hackatime_controller.rb	Replaces inline direct heartbeat ingestion logic with HeartbeatIngest.
test/services/heartbeat_ingest_test.rb	Adds service tests for direct ingestion, duplicate handling, language carry-forward, and import deduplication.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.