[Nachiket-Roy]

What changed and why

This pull request improves the progress reporting and filtering logic in the OSV advisory ingestion process, and enhances the related tests to verify correct progress event emission.

Improvements to advisory ingestion and progress reporting:

• The ingestAdvisories function now tracks both the number of processed entries and the number of valid advisories, ensuring that progress updates reflect all entries being processed, not just those passing filters. Progress events now use the total number of entries and the current processed count, improving accuracy and clarity.

Enhancements to test coverage:

• The test for syncOsvAdvisories now captures progress events and asserts that a progress update is emitted when all advisories have been processed, verifying the improved progress reporting logic.

Closes #1656

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Sync progress now reflects every processed archive record, including invalid and withdrawn advisories. Only eligible advisories are stored and included in the advisory count.

I updated OSV ingestion progress to count every archive entry, including withdrawn or otherwise unusable records. The ingest operation still stores only eligible advisories.

Changes: OSV ingestion progress

Layer / File(s)	Summary
Count processed archive entries src/advisory/osv-sync.ts, tests/osv-sync.test.ts	Progress reports processed archive entries rather than only accepted advisories. The test checks that processing two entries reports 2 / 2, while only the active advisory is stored.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to 6d5cf

This change makes ingest progress reach completion when withdrawn or unusable OSV records are skipped. Stored advisories are unchanged. The change is small and low risk to merge.

Pre-merge checks: 7

Check name	Status	Explanation
Title check		The title uses the required "type: short description" form with the allowed type "fix", lowercase after the colon, and no trailing period. It clearly names the main change: progress events now fire wh…
Description check		The description covers the required "What changed and why" section. It explains that ingestAdvisories now tracks processed entries separately from valid advisories, so progress reflects skipped record…
Linked Issues check		I checked the diff for src/advisory/osv-sync.ts. Issue #1656 has one coding requirement. The progress counter must reach the total when entries are skipped. Before this change, advisoryCount counted o…
Out of Scope Changes check		Every change maps to issue #1656. The osv-sync.ts hunk changes only ingestAdvisories. It adds processedEntries and totalAdvisories and moves the progress reporting to them. It also merges the three sk…
Linked Issue		I checked the PR body for an issue reference. The contributor's description ends with "Closes #1656". That matches the required keyword form (Closes) followed by an issue number. The linked issue is #…
Closes Keyword Matches Scope		I checked the PR body. It uses "Closes #1656", so the check applies. Issue #1656 asks for one thing. The progress counter in ingestAdvisories counts only accepted records, but the total counts all e…
No New Runtime Dependency		This change does not touch package.json. The diff inventory lists only two modified files: src/advisory/osv-sync.ts and tests/osv-sync.test.ts. I compared package.json at the base and head refs. The "…

Full details: Title check

Explanation

The title uses the required "type: short description" form with the allowed type "fix", lowercase after the colon, and no trailing period. It clearly names the main change: progress events now fire when records are skipped during ingest. The scope "(advisory)" matches the changed module.

Full details: Description check

Explanation

The description covers the required "What changed and why" section. It explains that ingestAdvisories now tracks processed entries separately from valid advisories, so progress reflects skipped records. It also describes the test update. It includes "Closes #1656", which links the linked issue. The template's placeholder comment was left in place, and the description has no separate note on whether any behavior outside progress reporting changed, but the content is mostly complete.

Full details: Linked Issues check

Explanation

I checked the diff for src/advisory/osv-sync.ts. Issue #1656 has one coding requirement. The progress counter must reach the total when entries are skipped. Before this change, advisoryCount counted only accepted records, and the completion check compared it to advisoryEntries.length. Now processedEntries increments for every archive entry, before any filter runs. The completion check is processedEntries === totalAdvisories, which holds on the last entry. Progress events report processedEntries and totalAdvisories. Skipped withdrawn, unaffected, and ID-less records therefore still advance the counter. The filter for id, nonempty affected, and no withdrawn is unchanged. Only valid records are pushed to parsedVulns and counted in advisoryCount. The test change in tests/osv-sync.test.ts adds an onProgress capture. It asserts an ingest event reporting 'Processing advisory records: 2 / 2' for a fixture with one withdrawn and one active advisory. That assertion covers the reported bug. No coding requirement from #1656 remains unmet.

Full details: Out of Scope Changes check

Explanation

Every change maps to issue #1656. The osv-sync.ts hunk changes only ingestAdvisories. It adds processedEntries and totalAdvisories and moves the progress reporting to them. It also merges the three skip checks into one condition. That merge keeps the same skip semantics. The test hunk adds progress capture and one assertion to the withdrawn-advisory ingest test. It supports the fix. The removed comment about OSV querybatch excluding withdrawn advisories is a small loss of documentation. It is not an unrelated change, and the withdrawn check it explained is still present. No unrelated file or behavior change appears in the reviewed diff.

Full details: Linked Issue

Explanation

I checked the PR body for an issue reference. The contributor's description ends with "Closes #1656". That matches the required keyword form (Closes) followed by an issue number. The linked issue is #1656, and the PR objectives confirm it is the directly linked issue. The check passes. From a maintainer's view: this PR fixes a progress bug in OSV ingestion. When withdrawn or unaffected records were skipped, the final progress event never fired. The body links that bug correctly, so the check is satisfied.

Full details: Closes Keyword Matches Scope

Explanation

I checked the PR body. It uses "Closes #1656", so the check applies. Issue #1656 asks for one thing. The progress counter in ingestAdvisories counts only accepted records, but the total counts all entries. When withdrawn or unaffected records are skipped, the final progress event never fires, and progress stops short (for example, 12000 / 14000). The requested outcome is that progress reaches completion despite skipped records. The diff in src/advisory/osv-sync.ts delivers that outcome: - A new processedEntries counter increments on every archive entry, before any filtering. - The progress condition now compares processedEntries with totalAdvisories (advisoryEntries.length). The final event fires when all entries are processed, so skipped records no longer block it. - The progress message and processedAdvisories field report processedEntries / totalAdvisories. - Filtering is unchanged in effect. A record still enters parsedVulns and increments advisoryCount only when it has an ID, a nonempty affected array, and no withdrawn value. The three continue checks are merged into one condition with the same meaning. The test change in tests/osv-sync.test.ts captures progress events. It asserts that the ingest event reports Processing advisory records: 2 / 2 for a fixture that includes a withdrawn advisory. This covers the reported case. The only item in the issue is delivered, so merging will not auto-close the issue with work left unbuilt.

Full details: No New Runtime Dependency

Explanation

This change does not touch package.json. The diff inventory lists only two modified files: src/advisory/osv-sync.ts and tests/osv-sync.test.ts. I compared package.json at the base and head refs. The "dependencies" block is identical in both: better-sqlite3, fflate, semver, yaml, and yarn-lockfile. "optionalDependencies" is absent in both. "devDependencies" is also identical. No package gained a runtime entry, and none moved from devDependencies into dependencies. The check passes. I am a maintainer here, and I want to be clear about scope. This PR fixes the ingest progress bug from #1656. It counts every archive entry, so the final progress event fires even when withdrawn or unaffected records are skipped. Users see progress reach completion. No new runtime package enters the project with this change.

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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Automatic approval policy

Commit: 6d5cf3432a7d.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Moderate	Global configuration
Security risk	Not set	Global configuration

No overrides matched. The global limits apply.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: OWASP/cve-lite-cli
• Branch: main
• File: .coderabbit.yaml
• Commit: bd4b051
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1ba79b04-4e38-4cff-8558-125ffce40cf6

Commits

Reviewing files that changed from the base of the PR and between bd4b051 and 6d5cf34.

Files selected for processing: 2

• src/advisory/osv-sync.ts
• tests/osv-sync.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.