fix: resolve code quality issues from simplify review#59
Conversation
- resolveUpdateMethod now delegates to isEmptyValue instead of duplicating a partial empty-check (fixes array inconsistency) - truncateHeadTail guards ellipsis separator like other truncation fns - truncateHeading guards empty body when remainingTokens is 0 - getContent early-returns when limit <= 0, avoiding unnecessary tokenization - Remove dead error-check branch in generateMetadata Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors/cleans up truncation and metadata-generation helpers to make behavior more consistent (especially around “empty” values and token limits) and removes redundant logic.
Changes:
- Adjusts truncation helpers (
truncateHeadTail,truncateHeading) to better handle edge cases around separators and empty bodies. - Optimizes
getContentby early-returning whenlimit <= 0to avoid unnecessary tokenization. - Simplifies
resolveUpdateMethodby delegating toisEmptyValue, and simplifiesgenerateMetadataerror handling.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/utils.ts | Updates truncation logic and adds limit <= 0 fast-path in getContent. |
| src/utils.test.ts | Tweaks truncateHeadTail expectations to align with updated separator behavior. |
| src/metadata.ts | Uses isEmptyValue in resolveUpdateMethod; simplifies generateMetadata catch branch. |
| main.js | Rebuilt bundle reflecting the TypeScript source changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…g tests - truncateHeadTail early-returns joinTokens(tokens) when limit >= length, preventing overlap and spurious separator insertion - Add tests: truncateHeadTail with limit > token count, truncateHeading with outline consuming full budget, getContent with limit=0 and limit=-1 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@copilot code review[agent] look now |
|
@philoserf I've opened a new pull request, #60, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot we addressed the items you raised. how does it look now? |
|
@philoserf I've opened a new pull request, #61, to work on those changes. Once the pull request is ready, I'll request review from you. |
Summary
resolveUpdateMethodnow delegates toisEmptyValueinstead of duplicating a partial empty-check (fixes array inconsistency)truncateHeadTailguards ellipsis separator like other truncation functionstruncateHeadingguards empty body whenremainingTokensis 0getContentearly-returns whenlimit <= 0, avoiding unnecessary tokenizationgenerateMetadataTest plan
🤖 Generated with Claude Code