Skip to content

fix(entities): count actual inserted rows in recordEntities on conflict (#27) - #28

Merged
7vignesh merged 1 commit into
7vignesh:mainfrom
DYNOSuprovo:fix/record-entities-conflict-count
Sep 21, 2026
Merged

7vignesh merged 1 commit into
7vignesh:mainfrom
DYNOSuprovo:fix/record-entities-conflict-count

Conversation

@DYNOSuprovo

Copy link
Copy Markdown
Contributor

Description

Fixes #27

Summary of Changes

  • In src/lib/entities.ts, updated recordEntities to count actual rows written using result.changes from insert.run(memoryRowId, entity, kind) instead of unconditionally incrementing recorded += 1.
  • When an entity matches multiple categories (e.g. redis, docker, kubernetes, which exist in both KNOWN_PACKAGES and KNOWN_SERVICES), extractEntities emits multiple entries with the same entity name. Because memory_entities has PRIMARY KEY (memory_rowid, entity), subsequent inserts hit ON CONFLICT DO NOTHING and return changes: 0. Tracking result.changes ensures the returned count accurately reflects rows written into SQLite.
  • In tests/entities.test.ts, added a regression test recordEntities counts actual inserted rows, not no-op conflicts verifying that multi-kind entities with ON CONFLICT DO NOTHING return the exact count matching rows stored in the database.

Verification

  • npm run typecheck: Passed with 0 errors.
  • tsx --test tests/*.test.ts: All 232 tests passed (including the new regression test).
  • npm run build: Successful build (dist/index.js, dist/cli.js).
  • npm run smoke: Smoke test passed cleanly.

…ct (7vignesh#27)

In recordEntities, the loop previously incremented recorded += 1 unconditionally for every entity in the loop, even when the insert statement executed ON CONFLICT DO NOTHING.

For multi-kind matches (such as entities recognized as both a package and a service like redis, docker, or kubernetes) which share the same (memory_rowid, entity) PRIMARY KEY, conflict no-ops caused the returned count to overstate the rows actually written.

Use the statement result changes property (recorded += result.changes) to track actual inserts and add a regression test.

Fixes 7vignesh#27
@vercel

vercel Bot commented Sep 20, 2026

Copy link
Copy Markdown

@DYNOSuprovo is attempting to deploy a commit to the vignesh's projects Team on Vercel.

A member of the Team first needs to authorize it.

@7vignesh 7vignesh left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean one-line fix, and the test surfaced something I underestimated when filing #27 - redis/docker/kubernetes live in both KNOWN_PACKAGES and KNOWN_SERVICES, and the table PK is (memory_rowid, entity) without kind, so the conflict is actually reachable, not just theoretical. Verified locally: typecheck clean, all entity tests pass, green on Node 20/22. Thanks @DYNOSuprovo!

@7vignesh
7vignesh merged commit 8542806 into 7vignesh:main Sep 21, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

recordEntities returns an inflated count when inserts hit ON CONFLICT DO NOTHING

2 participants