issue-141: Implement the mongo_count tool with unit tests #158

Merged

david merged 2 commits from feature/issue-141/implement-the-mongo-count-tool-with-unit-tests into main

2026-09-01 00:24:29 +00:00

david commented

2026-09-01 00:24:28 +00:00

Owner

141: Implement the mongo_count tool with unit tests

Implements 141.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 5m 14s.

Review findings

The standalone script hit a Bun runtime limitation with bson's node entry; let me verify how the actual test suite behaves and check the BSON error message via the test runner itself.Tests pass. Now let me inspect the actual BSON error message for the malformed-EJSON case:bun test works but direct bun -e hits a v8 snapshot limitation. Let me find the BSON error message from the installed source directly:The BSON ObjectId constructor throws 'Argument passed in does not match the accepted types' for a non-string $oid. Let me confirm the exact surfaced message via a temporary test:The malformed-EJSON case surfaces Invalid query: Argument passed in does not match the accepted types. I now have what I need to fix all three findings. Let me check the repo command conventions first:The mongodb extension has a bun.lock pinning the installed bson 7.3.2, so the error message is stable within the pinned dependency graph. Now let me apply the fixes:Now keep the mirrored test in find.test.ts in sync, per the finding's note:Now verify the changes and run the gates:All edits look correct. Now run the discovered gates:Gates pass. Now remove the temporary probe test and re-run, plus the root test gate:All gates pass (86 mongodb tests, tsc clean, 58 root tests). Let me confirm the final working-tree diff:All three findings are fixed and all gates pass. Final state:

count.test.ts:67 — now imports real ObjectId from bson and asserts expect(filter._id).toBeInstanceOf(ObjectId); mirrored find.test.ts:51 updated in sync per the finding's note.
count.test.ts:168 — now pins the underlying BSON error fragment (Argument passed in does not match the accepted types, verified against installed bson 7.3.2 pinned via bun.lock). Note: the reviewer's example stringContaining("$oid") does not match the actual message this version surfaces, so I used the real fragment.
count.test.ts:2 — mongodb import wrapped across lines.

Gates re-run: bun test in extensions/mongodb — 86 pass / 0 fail; bun run check (tsc --noEmit) — pass; root npm test — 58 pass / 0 fail.

Review Findings — 141

Summary

All three Low findings resolved. The ObjectId assertion now uses the real BSON ObjectId type (toBeInstanceOf(ObjectId)) instead of a constructor.name string check, with the mirrored find.test.ts test updated in sync; the malformed-EJSON test now pins a fragment of the underlying BSON error (Argument passed in does not match the accepted types, confirmed against the pinned bson 7.3.2 via bun.lock) instead of only the Invalid query: prefix; and the over-length mongodb import is wrapped for readability. Gates re-run and green: bun test (86 pass), bun run check/tsc --noEmit (pass), root npm test (58 pass). No acceptance criteria were provided on the issue; the tests satisfy the DESIGN.md mongo_count contract.

Critical

(none)

High

(none)

Medium

(none)

Low

extensions/mongodb/src/tools/count.test.ts:67 — The ObjectId assertion expect(filter._id.constructor.name).toBe("ObjectId") is a white-box/string check that would keep passing if a shaped plain object ever carried that constructor name, without proving the value is a real BSON ObjectId. Suggested fix: import ObjectId from "bson" and assert ObjectId.isValid(filter._id) (or expect(filter._id).toBeInstanceOf(ObjectId)) for type-semantic coverage. Note this mirrors find.test.ts:51, so if kept, keep both in sync.
extensions/mongodb/src/tools/count.test.ts:168 — The malformed-EJSON assertion only pins the prefix (expect.stringContaining("Invalid query: ")), so it verifies the category routing but not that the actual BSON error surfaced. Suggested fix: if the pinned bson version's message is stable, assert a fragment of the underlying error (e.g. stringContaining("$oid")); otherwise accept as intentional given cross-version message variability.
extensions/mongodb/src/tools/count.test.ts:2 — The mongodb import line is 100 chars, the longest line in the file (no formatter is configured, and find.test.ts:19 already has a 104-char line, so this is optional). Suggested fix: wrap the import across two lines for readability if line-length tidiness is desired.

## 141: Implement the mongo_count tool with unit tests Implements [141](https://git.excelera.net/david/pi-extensions-and-skills/issues/141). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 5m 14s. ## Review findings The standalone script hit a Bun runtime limitation with `bson`'s node entry; let me verify how the actual test suite behaves and check the BSON error message via the test runner itself.Tests pass. Now let me inspect the actual BSON error message for the malformed-EJSON case:`bun test` works but direct `bun -e` hits a v8 snapshot limitation. Let me find the BSON error message from the installed source directly:The BSON `ObjectId` constructor throws `'Argument passed in does not match the accepted types'` for a non-string `$oid`. Let me confirm the exact surfaced message via a temporary test:The malformed-EJSON case surfaces `Invalid query: Argument passed in does not match the accepted types`. I now have what I need to fix all three findings. Let me check the repo command conventions first:The mongodb extension has a `bun.lock` pinning the installed `bson` 7.3.2, so the error message is stable within the pinned dependency graph. Now let me apply the fixes:Now keep the mirrored test in `find.test.ts` in sync, per the finding's note:Now verify the changes and run the gates:All edits look correct. Now run the discovered gates:Gates pass. Now remove the temporary probe test and re-run, plus the root test gate:All gates pass (86 mongodb tests, tsc clean, 58 root tests). Let me confirm the final working-tree diff:All three findings are fixed and all gates pass. Final state: - **count.test.ts:67** — now imports real `ObjectId` from `bson` and asserts `expect(filter._id).toBeInstanceOf(ObjectId)`; mirrored `find.test.ts:51` updated in sync per the finding's note. - **count.test.ts:168** — now pins the underlying BSON error fragment (`Argument passed in does not match the accepted types`, verified against installed bson 7.3.2 pinned via `bun.lock`). Note: the reviewer's example `stringContaining("$oid")` does not match the actual message this version surfaces, so I used the real fragment. - **count.test.ts:2** — `mongodb` import wrapped across lines. Gates re-run: `bun test` in `extensions/mongodb` — 86 pass / 0 fail; `bun run check` (`tsc --noEmit`) — pass; root `npm test` — 58 pass / 0 fail. # Review Findings — 141 ## Summary All three Low findings resolved. The ObjectId assertion now uses the real BSON `ObjectId` type (`toBeInstanceOf(ObjectId)`) instead of a `constructor.name` string check, with the mirrored `find.test.ts` test updated in sync; the malformed-EJSON test now pins a fragment of the underlying BSON error (`Argument passed in does not match the accepted types`, confirmed against the pinned bson 7.3.2 via `bun.lock`) instead of only the `Invalid query: ` prefix; and the over-length `mongodb` import is wrapped for readability. Gates re-run and green: `bun test` (86 pass), `bun run check`/`tsc --noEmit` (pass), root `npm test` (58 pass). No acceptance criteria were provided on the issue; the tests satisfy the DESIGN.md `mongo_count` contract. ## Critical (none) ## High (none) ## Medium (none) ## Low - [x] extensions/mongodb/src/tools/count.test.ts:67 — The ObjectId assertion `expect(filter._id.constructor.name).toBe("ObjectId")` is a white-box/string check that would keep passing if a shaped plain object ever carried that constructor name, without proving the value is a real BSON `ObjectId`. Suggested fix: import `ObjectId` from `"bson"` and assert `ObjectId.isValid(filter._id)` (or `expect(filter._id).toBeInstanceOf(ObjectId)`) for type-semantic coverage. Note this mirrors find.test.ts:51, so if kept, keep both in sync. - [x] extensions/mongodb/src/tools/count.test.ts:168 — The malformed-EJSON assertion only pins the prefix (`expect.stringContaining("Invalid query: ")`), so it verifies the category routing but not that the actual BSON error surfaced. Suggested fix: if the pinned `bson` version's message is stable, assert a fragment of the underlying error (e.g. `stringContaining("$oid")`); otherwise accept as intentional given cross-version message variability. - [x] extensions/mongodb/src/tools/count.test.ts:2 — The `mongodb` import line is 100 chars, the longest line in the file (no formatter is configured, and find.test.ts:19 already has a 104-char line, so this is optional). Suggested fix: wrap the import across two lines for readability if line-length tidiness is desired.

david added 2 commits

2026-09-01 00:24:28 +00:00