Skip to content

Conversation

@fivecar
Copy link
Owner

@fivecar fivecar commented Sep 25, 2025

This sends onBegin and onDone during init for files which have already been completed in a
previous session

This sends `onBegin` and `onDone` during init for files which have already been completed in a
previous session
@coderabbitai
Copy link

coderabbitai bot commented Sep 25, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Pre-commit hook now runs tests after lint-staged. README clarifies init lifecycle: onBegin/onDone are invoked for pre-downloaded files. Jest adds 100% coverage thresholds and collection config. Core logic updates handle finished specs by verifying on-disk files, emitting onBegin/onDone, or restarting downloads. Tests cover these scenarios.

Changes

Cohort / File(s) Summary
Repo Hooks & Coverage
\.husky/pre-commit, jest.config.js
Pre-commit runs npm test after lint-staged. Jest configured to collect coverage from src/**/*.{js,jsx,ts,tsx} (excluding d.ts, lib, node_modules) with global 100% thresholds for branches, functions, lines, statements.
Documentation
README.md
Clarifies that during init, onBegin and onDone are also invoked for files already downloaded in previous sessions. Updates handler descriptions accordingly.
Core Download Logic
src/index.ts
Adds guard to skip specs with createTime <= 0. For finished specs, stats file: if exists, emit onBegin then onDone; if missing, unset finished, persist, and restart download. In reviveTask, early return when no spec; handles finished-spec revival by notifying or restarting based on file presence. Ensures consistent behavior for lazy-deleted/legacy states.
Tests
test/index.spec.ts
Adds tests for finished specs across scenarios: with/without tasks, lazy-deleted, file present/missing. Verifies correct onBegin/onDone emissions and that downloads restart when files are missing.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App as App Init
  participant Q as DownloadQueue
  participant S as Spec Store
  participant FS as File System
  participant H as Handlers

  App->>Q: initialize()/resume()
  Q->>S: load specs
  loop for each spec
    alt spec.createTime <= 0
      Q-->>Q: skip (lazy-deleted)
    else spec.finished == true
      Q->>FS: stat(spec.filePath)
      alt file exists
        rect rgb(230,245,255)
        note over Q,H: Notify finished spec discovered during init
        Q-->>H: onBegin(spec)
        Q-->>H: onDone(spec)
        end
      else file missing
        Q-->>Q: spec.finished = false
        Q->>S: persist(spec)
        Q-->>Q: start download(spec)
      end
    else not finished
      Q-->>Q: start download(spec)
    end
  end
Loading
sequenceDiagram
  autonumber
  actor Sys as Resume Existing Task
  participant Q as DownloadQueue
  participant S as Spec Store
  participant FS as File System
  participant H as Handlers

  Sys->>Q: reviveTask(taskId)
  Q->>S: get spec by task
  alt no spec
    Q-->>Sys: return
  else spec.finished == true and spec.createTime > 0
    Q->>FS: stat(spec.filePath)
    alt file exists
      rect rgb(230,245,255)
      note over Q,H: Revived finished spec
      Q-->>H: onBegin(spec)
      Q-->>H: onDone(spec)
      end
    else missing
      Q-->>Q: spec.finished = false
      Q->>S: persist(spec)
      Q-->>Q: restart download(spec)
    end
  else proceed with normal continuation
    Q-->>Q: resume task/download
  end
Loading
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-launch-finished-notifications

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 36306bf and cc230af.

📒 Files selected for processing (5)
  • .husky/pre-commit (1 hunks)
  • README.md (2 hunks)
  • jest.config.js (1 hunks)
  • src/index.ts (2 hunks)
  • test/index.spec.ts (2 hunks)

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@fivecar fivecar merged commit 61ece4f into main Sep 25, 2025
2 checks passed
@fivecar fivecar deleted the update-launch-finished-notifications branch September 25, 2025 00:03
@github-actions
Copy link

🎉 This PR is included in version 5.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants