chore: Phase 6 maintenance — tooling, deps, lint cleanup#38
Conversation
- Fix lint/format scripts to use eslint/prettier (was echo stubs) - Remove orphan CHANGELOG entries (duplicate 0.8.1/0.9.2) - Update devDeps: @pi-* 0.79.3->0.80.2, typescript 5.4.0->5.9.3 - Resolve npm audit (0 vulns remaining) - Add .npmignore for clean package publishing - Fix 7 unused-variable lint warnings - Add ROADMAP Phase 6: Maintenance & Developer Tooling - Bump version to 1.1.0 Closes: ROADMAP 6.1-6.5
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 shouldCompact Gate 2 (hardWatermark) is unreachable
In extensions/engine.ts:436-449, shouldCompact() has two gates: Gate 1 fires at contextWindow * 0.6 (60%), and Gate 2 fires at contextWindow * hardWatermark (default 0.5, so 50%). Since 60% > 50%, Gate 1 will always trigger first for any token count that would trigger Gate 2. Gate 2 is dead code unless hardWatermark is configured above 0.6, which contradicts the name 'hard watermark' (typically a higher, not lower, threshold). This is a pre-existing issue not introduced by this PR, but it's worth noting since the JSDoc says 'whichever hits first' implying they serve different purposes.
(Refers to lines 436-449)
Was this helpful? React with 👍 or 👎 to provide feedback.
| - 0 errors, 25 warnings (all pre-existing `any` type hints) | ||
| - Prettier formatting clean across all source | ||
|
|
||
| l). |
There was a problem hiding this comment.
🟡 Stray garbage text appears in the published changelog
A truncated fragment l). is left on its own line in the changelog (CHANGELOG.md:34), between the Quality Gates section and the 1.0.0 heading, so users and npm package viewers see nonsensical text.
Impact: The published CHANGELOG visible on npm and GitHub contains corrupted/nonsensical text.
Appears to be a copy-paste artifact
Line 34 of CHANGELOG.md contains just l). which looks like the tail end of a sentence that was accidentally left behind when editing. It sits right after the "Quality Gates" bullet list for v1.1.0 and before the ## [1.0.0] heading. Since CHANGELOG.md is listed in the package.json:files array (package.json:57), this garbage text will ship in the npm package.
| l). |
Was this helpful? React with 👍 or 👎 to provide feedback.
| protected: protectedMsgs, | ||
| compressible, | ||
| discardable: _discardable, | ||
| // discardable: _discardable, |
There was a problem hiding this comment.
🟡 Commented-out code left in production source violates repo rules
A destructured variable is commented out rather than removed (// discardable: _discardable, at extensions/engine.ts:475), instead of simply omitting it from the destructuring, so commented-out code ships in production source.
Impact: Violates the repository's explicit "no commented-out code in commits" rule.
CONTRIBUTING.md rule violation detail
CONTRIBUTING.md states: "No debug logs, commented-out code, or TODOs in commits". The previous code destructured discardable: _discardable from classifyMessages(). The fix should simply omit the property from the destructuring pattern entirely, since the return value is unused.
| // discardable: _discardable, |
Was this helpful? React with 👍 or 👎 to provide feedback.
| ]; | ||
| // Budget allows user message but not the full tool output | ||
| const totalTokens = engine.estimateTokens(msgs); | ||
| // const totalTokens = engine.estimateTokens(msgs); |
There was a problem hiding this comment.
🟡 Commented-out code left in test file violates repo rules
An unused variable assignment is commented out rather than deleted (// const totalTokens = ... at tests/engine-coverage.test.ts:937), instead of removing the line, so commented-out code remains in the codebase.
Impact: Violates the repository's explicit "no commented-out code in commits" rule.
CONTRIBUTING.md rule violation detail
CONTRIBUTING.md states: "No debug logs, commented-out code, or TODOs in commits". The variable totalTokens was unused, so the correct fix is to delete the line entirely rather than commenting it out.
| // const totalTokens = engine.estimateTokens(msgs); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| }, | ||
| }; | ||
| } catch (error) { | ||
| } catch { |
There was a problem hiding this comment.
🚩 Catch clause drops error context for diagnostics
The change from catch (error) to catch at extensions/index.ts:244 is valid since the error variable was indeed unused in the catch block body. However, this means any diagnostic information from the thrown error (e.g., from extensions/index.ts:225 where an explicit new Error('Empty summary...') is thrown, or from failures inside engine.generateSummary) is silently discarded. The notification at line 249 only reports the failure count, not the root cause. For a production extension, capturing and logging the error message would aid debugging compaction failures.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Phase 6 of the ROADMAP: Maintenance & Developer Tooling. Five of six tasks completed.
Changes
Quality Gates
Remaining (6.6)
Bump version to 1.1.0 — done in this PR.