feat: Pre-download bundle uploads before lock to reduce contention

[jason-ford-codecov]

• Download upload files before acquiring commit-level lock
• Reduces lock hold time by ~30-50% per bundle
• Add lock_key_suffix to LockManager for future per-bundle locking
• Add extract_bundle_name_from_file helper for future use

---

Note

Medium Risk
Touches bundle-analysis processing and lock acquisition order, which can impact concurrency behavior and temp-file lifecycle if edge cases are missed, but changes are mostly additive and covered by new tests.

Overview
Reduces bundle-analysis lock contention by pre-downloading upload artifacts before acquiring the commit-level lock, then passing a local file path into processing.

BundleAnalysisReportService.process_upload now accepts an optional pre_downloaded_path to skip the GCS read and avoid deleting caller-managed temp files, while BundleAnalysisProcessorTask introduces a BundleUploadFile context manager to do the pre-download and ensure temp-file cleanup. The processor also adds a guard to reject uploads whose commit doesn’t match the task’s commit.

Test coverage is expanded to verify the pre-download path is used (and storage download is skipped), error cases return None from pre-download, and a benchmark-style unit test asserts reduced lock hold time; .gitignore also adds .cursor/.

^{Written by Cursor Bugbot for commit fbd2c39. This will update automatically on new commits. Configure here.}

[sentry]

Codecov Report

❌ Patch coverage is 96.92308% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.45%. Comparing base (558d4d8) to head (2cb4b58).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
apps/worker/tasks/bundle_analysis_processor.py	96.29%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #678      +/-   ##
==========================================
+ Coverage   92.43%   92.45%   +0.01%     
==========================================
  Files        1297     1297              
  Lines       47730    47762      +32     
  Branches     1606     1606              
==========================================
+ Hits        44119    44156      +37     
+ Misses       3302     3297       -5     
  Partials      309      309              

Flag	Coverage Δ
workerintegration	58.62% <75.38%> (+0.02%)	⬆️
workerunit	90.38% <96.92%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[codecov-notifications]

Codecov Report

❌ Patch coverage is 96.92308% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
apps/worker/tasks/bundle_analysis_processor.py	96.29%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[drazisil-codecov]

AI Code Review

Summary

This PR reduces lock contention by pre-downloading bundle upload files before acquiring the Redis lock. The implementation is well-designed with proper cleanup handling and error cases.

Status: APPROVED with minor suggestions

---

Issues Found

Issue 1: Misleading comment (Low Priority)

File: apps/worker/services/bundle_analysis/report.py
Function: extract_bundle_name_from_file
Problem: Comment states "Stop after reading the first ~100 events" but no limit is implemented. The loop continues until bundleName is found or EOF.

Suggested Fix:

# Option A: Implement the limit
for count, (prefix, event, value) in enumerate(ijson.parse(f)):
    if prefix == "bundleName":
        return value
    if count > 100:
        break

# Option B: Fix the comment
# Returns early when bundleName is found; returns None if not present

Action Required: FIX_COMMENT_OR_IMPLEMENT_LIMIT

Issue 2: Unused code added (Low Priority, Intentional)

Files: apps/worker/services/lock_manager.py, apps/worker/services/bundle_analysis/report.py
Problem: lock_key_suffix parameter and extract_bundle_name_from_file helper are added but not used in this PR.
Note: PR description indicates this is intentional groundwork for future per-bundle parallelism.

Action Required: NONE (defer to author discretion)

---

Verification Checklist

• Cleanup handling is correct (finally blocks properly clean up temp files)
• Race conditions addressed (each worker creates own temp file copy)
• Fallback behavior works when pre-downloaded file is missing
• Error handling in _pre_download_upload_file is comprehensive
• Test coverage is adequate

---

For AI Agents

{
  "review_result": "APPROVED_WITH_SUGGESTIONS",
  "blocking_issues": [],
  "non_blocking_issues": [
    {
      "id": "misleading-comment",
      "severity": "low",
      "file": "apps/worker/services/bundle_analysis/report.py",
      "function": "extract_bundle_name_from_file",
      "action": "Either implement 100-event limit or update comment to reflect actual behavior",
      "lines": "24-25"
    }
  ],
  "approved": true
}

[thomasrockhu-codecov]

We might be able to take advantage of creating a class that handles this file creation and removal.

that way, we can use the pattern (please think of a better class name)

class BundleAnalysisFile:
    def __enter__(self):
        # do all the downloading stuff here

    def __exit__(self):
        # do all the temp file deletion here


with BundleAnalysisFile as ba:
    with Lockwhateverdo the lock stuff:
        do stuff

see https://stackoverflow.com/questions/3774328/implementing-use-of-with-object-as-f-in-custom-class-in-python

using a contextlib is also a good solution if we don't want to make a class

https://docs.python.org/3/library/contextlib.html

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[cursor]

Test imports non-existent temporary_upload_file function

High Severity

The test file imports temporary_upload_file from tasks.bundle_analysis_processor, but this name doesn't exist in the module. The context manager class is actually named BundleUploadFile. This naming mismatch will cause an ImportError when running any of the new tests that use temporary_upload_file (at lines 2058, 2089, 2101, 2113, and 2141).