Add a heartbeat remapping pipeline - #1686
skyfallwastaken wants to merge 2 commits into
Conversation
ce764f0 to
8bc51ba
Compare
Greptile SummaryAdds a shared heartbeat remapping pipeline for live and imported ingestion, durable hash aliases for deduplication compatibility and an auditable historical correction workflow with rollback support.
Confidence Score: 4/5The PR is not yet safe to merge because the historical runner still has an unresolved failure-state defect and two outstanding repository-rule violations. Rollback batches still lack error handling that records a terminal failure, so an unexpected rollback exception leaves the run indefinitely in Files Needing Attention: app/services/heartbeat_remap_runner.rb Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Client[Heartbeat client or import] --> Normalize[Normalize heartbeat]
Normalize --> Remap[Apply ordered remapping rules]
Remap --> Hash[Generate canonical identity hash]
Hash --> Resolve{Stored hash or alias exists?}
Resolve -- Yes --> Duplicate[Return existing heartbeat]
Resolve -- No --> Persist[Persist canonical heartbeat]
Persist --> Alias[Store pre-remap and legacy hash aliases]
Persist --> Rollup[Schedule dashboard rollup refresh]
Historical[Historical remap run] --> Remap
Historical --> Audit[Store preimage, postimage and alias changes]
Audit --> Rollback[Optional guarded rollback]
Reviews (4): Last reviewed commit: "Install Compose in orbs with an existing..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def self.rollback_batch!(run) | ||
| affected_user_ids = [] | ||
|
|
||
| HeartbeatRemapRun.transaction do |
There was a problem hiding this comment.
If any exception escapes a rollback batch, the transaction is reverted but the run remains rolling_back without a recorded error or terminal state. The job retries only the runner's three transient exceptions, so other SQL, validation, serialization or programming failures leave the recovery workflow appearing active indefinitely. Re-enqueuing the run then repeats the same unrecorded failure. Handle rollback errors consistently with process_batch! so operators can see and recover from a failed rollback.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/services/heartbeat_remap_runner.rb
Line: 95
Comment:
**Rollback failures strand runs**
If any exception escapes a rollback batch, the transaction is reverted but the run remains `rolling_back` without a recorded error or terminal state. The job retries only the runner's three transient exceptions, so other SQL, validation, serialization or programming failures leave the recovery workflow appearing active indefinitely. Re-enqueuing the run then repeats the same unrecorded failure. Handle rollback errors consistently with `process_batch!` so operators can see and recover from a failed rollback.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| heartbeats = Heartbeat.with_deleted.where(id: ids).order(:id).lock.to_a | ||
| users_by_id = User.where(id: heartbeats.map(&:user_id)).index_by(&:id) | ||
| plans = heartbeats.filter_map { |heartbeat| build_plan(heartbeat, users_by_id[heartbeat.user_id]) } | ||
| collision_rows = Heartbeat.where(fields_hash: plans.map { |plan| plan[:post_hash] }.uniq) | ||
| .where.not(id: plans.map { |plan| plan[:heartbeat].id }).order(:id).lock.to_a |
There was a problem hiding this comment.
The historical job explicitly locks the run plus candidate and collision heartbeat rows, then keeps those locks while it builds plans and writes audit and alias records. This violates the repository directive that jobs must not lock the database and can make concurrent heartbeat ingestion or updates wait for the full batch transaction. This repository requirement must be satisfied before merging by reducing the lock scope or using a non-blocking claim and check strategy.
Context Used: AGENTS.md (source)
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/services/heartbeat_remap_runner.rb
Line: 37-41
Comment:
**Job holds database locks**
The historical job explicitly locks the run plus candidate and collision heartbeat rows, then keeps those locks while it builds plans and writes audit and alias records. This violates the repository directive that jobs must not lock the database and can make concurrent heartbeat ingestion or updates wait for the full batch transaction. This repository requirement must be satisfied before merging by reducing the lock scope or using a non-blocking claim and check strategy.
**Context Used:** AGENTS.md ([source](https://github.com/hackclub/hackatime/blob/main/AGENTS.md))
**Knowledge Base Used:**
- [Remove the blocking Lapse heartbeat migration](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/reverts/revert_1058-20260311-blocking-lapse-migration-60db839.md)
- [Roll Back the blocking Lapse heartbeat migration](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/reverts/revert_1057-20260311-lapse-migration-app-blocking-1d69479.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
8bc51ba to
8a92fd9
Compare
8a92fd9 to
0811f7d
Compare
| rescue => error | ||
| run.update_columns( | ||
| state: HeartbeatRemapRun.states.fetch("failed"), | ||
| error_count: run.error_count + 1, | ||
| error_message: error.message, | ||
| finished_at: Time.current, | ||
| updated_at: Time.current | ||
| ) if run.persisted? | ||
| raise | ||
| end |
There was a problem hiding this comment.
The rescue => error handler catches every StandardError, including unexpected programming errors, updates the run to failed, and then re-raises. This violates the repository directive to rescue only specific failures that can be handled meaningfully and let unexpected failures surface directly. This repository requirement must be satisfied before merging by narrowing the handler to anticipated operational failures or using a mechanism that does not indiscriminately intercept every error.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/services/heartbeat_remap_runner.rb
Line: 66-75
Comment:
**Broad Rescue Hides Failures**
The `rescue => error` handler catches every `StandardError`, including unexpected programming errors, updates the run to `failed`, and then re-raises. This violates the repository directive to rescue only specific failures that can be handled meaningfully and let unexpected failures surface directly. This repository requirement must be satisfied before merging by narrowing the handler to anticipated operational failures or using a mechanism that does not indiscriminately intercept every error.
**Context Used:** AGENTS.md ([source](https://github.com/hackclub/hackatime/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Co-authored-by: Amp <amp@ampcode.com>
1 participant
Summary of the problem
WakaTime clients can submit incorrect language metadata, including classifying environment files as Ezhil. The existing Luau correction was a one-off ingest override and historical corrections had no reusable, auditable workflow.
Describe your changes
Add an ordered pure remapper used before canonical hashing in direct and imported ingestion. Preserve deduplication through durable hash aliases and add a resumable, auditable historical runner with dry-run, collision handling, rollback and coalesced rollup refreshes. Include an operations runbook for rollout and recovery.
This is PR 3 of 3 in a stacked change. It depends on #1685, which depends on #1684. Apply the remapping migration before starting updated web or worker processes.
Screenshots / Media
Not applicable. This is an ingestion and operations change.