test: add unit coverage for cost-tracker, compact, knowledge, tool-utils#71
Open
stealthwhizz wants to merge 1 commit into
Open
test: add unit coverage for cost-tracker, compact, knowledge, tool-utils#71stealthwhizz wants to merge 1 commit into
stealthwhizz wants to merge 1 commit into
Conversation
These core modules had no unit tests. Added deterministic, offline suites (node:test) covering their pure logic: - cost-tracker: session/per-model aggregation, derived totalTokens, defensive get() copy, reset - compact: token estimation, per-message-type accounting, compaction threshold, tool-result truncation, transcript rendering + compact prompt building - knowledge: index loading (always_load preload, available listing, missing files, malformed index) and prompt formatting - tool-utils: GCToolDefinition → AgentTool mapping, string vs object handler results, abort-signal forwarding All 26 new cases live under test/ so `npm test` runs them. Full suite: 53 passing.
shreyas-lyzr
approved these changes
Jul 6, 2026
shreyas-lyzr
left a comment
Contributor
There was a problem hiding this comment.
26 new test cases across four previously-uncovered modules. All look correct.
A few things I checked explicitly:
- estimateTokens arithmetic (ceil(5/4)=2), estimateMessageTokens summation (1+2+51+1=55), and the 75% compaction threshold (800/1000=0.8) all hold.
- totalTokens fallback (input+output when field absent) is tested correctly.
- The defensive-copy test for CostTracker.get() properly verifies internal state survives external mutation of the snapshot.
- knowledge tests use mkdtemp + rm recursive cleanup — no leaked temp dirs.
- before() blocks are correctly placed at module scope, not inside describe(), which is required for node:test.
- All suites are offline — no network, no API keys, no side effects.
The follow-up note in the PR description about test/tests not being wired into the npm test glob is a real gap worth tracking separately.
There was a problem hiding this comment.
Pull request overview
Adds deterministic node:test unit suites for several core “pure logic” modules so their behavior is exercised offline and without external dependencies.
Changes:
- Added new unit tests for
CostTrackersession + per-model aggregation behavior. - Added new unit tests for compaction/token estimation utilities, truncation logic, and transcript/prompt building.
- Added new unit tests for knowledge index loading/formatting and tool-definition conversion utilities.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
test/tool-utils.test.ts |
Tests toAgentTool mapping behavior, handler result normalization, and abort-signal forwarding. |
test/knowledge.test.ts |
Tests knowledge index loading (including malformed/missing cases) and prompt formatting output. |
test/cost-tracker.test.ts |
Tests CostTracker accumulation, per-model aggregation, defensive snapshotting, and reset behavior. |
test/compact.test.ts |
Tests token estimation, compaction thresholding, tool-result truncation, transcript rendering, and compact-prompt building. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Several core modules shipped with no unit tests. This PR adds deterministic, offline
node:testsuites for the pure logic in four of them — +26 test cases, all placed undertest/sonpm testactually runs them.Coverage before / after
Existing tests covered
sdk(tool helper, schema builder, programmatic hooks, query error paths) andtelemetry. These modules had none:cost-tracker.tstotalTokensfallback, defensiveget()copy,reset()compact.tsestimateTokensheuristic, per-message-type accounting (incl. tool_use overhead), 75% compaction threshold, tool-result head/tail truncation, transcript rendering (drops deltas/system noise), compact-prompt buildingknowledge.tsalways_loadpreload + trim, available listing, missing-file skip, malformed index → empty; prompt formatting (inline docs + read hint)tool-utils.tsGCToolDefinition → AgentToolmapping, string vs{text, details}handler results, abort-signal forwardingNotes
Follow-up worth flagging (not in this PR)
The
npm testscript globs onlytest/*.test.ts, so the existing suites undersrc/**/__tests__/(memory, sandbox-memory, skill-learner, task-tracker) andsrc/__tests__/telemetry.test.tsare never run by CI. Wiring those into the test script (or moving them undertest/) would be a good separate cleanup.