Skip to content

Commit 119220b

Browse files
jsonifyclaude
andauthored
Claude/fix summarize feature (#76)
* fix: include js-yaml dependency in extension package The js-yaml module was declared in dependencies but wasn't being bundled in the packaged extension. Updated the package script to copy js-yaml to out/node_modules/ during packaging. This fixes the "Cannot find module 'js-yaml'" error when using the generateTagsCurrentNote command (AI Summarize feature). Resolves issue with TagParser requiring js-yaml for YAML frontmatter parsing. * test: add comprehensive unit tests for TagParser Adds 17 unit tests covering: - hasFrontmatter() detection - parseTags() for array/list/string formats - writeTags() for adding/updating frontmatter - removeTags() edge cases These tests verify js-yaml integration and would catch dependency issues during development (but not packaging issues). * refactor: improve dependency bundling and test determinism 1. Replace hardcoded package script with bundle-deps.js: - Dynamic dependency resolution using require.resolve - Handles packages that don't export package.json - Cleaner package.json script - More maintainable (no version numbers hardcoded) 2. Improve TagParser tests with sinon fake timers: - Use fixed timestamp (2024-01-15T12:00:00.000Z) for deterministic tests - Verify exact tagged-at timestamp value, not just presence - Tests now properly validate timestamp behavior These improvements address code review feedback for better maintainability and test robustness. --------- Co-authored-by: Claude <[email protected]>
1 parent 160d995 commit 119220b

File tree

3 files changed

+357
-1
lines changed

3 files changed

+357
-1
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@
11891189
"pretest": "pnpm run compile",
11901190
"test": "node ./out/test/runTest.js",
11911191
"test:unit": "mocha --require out/test/setup.js out/test/unit/**/*.test.js",
1192-
"package": "mkdir -p out/node_modules && cp -rL node_modules/.pnpm/[email protected]/node_modules/marked out/node_modules/ && cp -rL node_modules/.pnpm/[email protected]/node_modules/markdown-it-regex out/node_modules/ && vsce package --no-dependencies",
1192+
"package": "node scripts/bundle-deps.js && vsce package --no-dependencies",
11931193
"changelog": "conventional-changelog -p angular -i CHANGELOG.md -s",
11941194
"release:patch": "standard-version --release-as patch && pnpm run package",
11951195
"release:minor": "standard-version --release-as minor && pnpm run package",

scripts/bundle-deps.js

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
#!/usr/bin/env node
2+
3+
/**
4+
* Bundle dependencies for VS Code extension packaging
5+
*
6+
* This script copies required runtime dependencies to out/node_modules/
7+
* so they're included in the packaged .vsix file when using --no-dependencies.
8+
*
9+
* This is necessary because vsce's --no-dependencies flag excludes all node_modules,
10+
* but we need certain runtime dependencies available in the extension.
11+
*/
12+
13+
const fs = require('fs');
14+
const path = require('path');
15+
16+
// Dependencies that need to be bundled with the extension
17+
const DEPS_TO_BUNDLE = [
18+
'marked',
19+
'markdown-it-regex',
20+
'js-yaml'
21+
];
22+
23+
const OUT_DIR = path.join(__dirname, '..', 'out', 'node_modules');
24+
25+
/**
26+
* Resolve the actual location of a package in pnpm's node_modules
27+
* @param {string} packageName - Name of the package to resolve
28+
* @returns {string} Absolute path to the package directory
29+
*/
30+
function resolvePackagePath(packageName) {
31+
try {
32+
// First try to resolve package.json directly
33+
try {
34+
const packageJsonPath = require.resolve(`${packageName}/package.json`, {
35+
paths: [path.join(__dirname, '..')]
36+
});
37+
return path.dirname(packageJsonPath);
38+
} catch (e) {
39+
// If package.json is not exported, resolve the main entry point instead
40+
const mainPath = require.resolve(packageName, {
41+
paths: [path.join(__dirname, '..')]
42+
});
43+
44+
// Walk up the directory tree to find the package root
45+
let currentDir = path.dirname(mainPath);
46+
while (currentDir !== path.dirname(currentDir)) {
47+
const potentialPackageJson = path.join(currentDir, 'package.json');
48+
if (fs.existsSync(potentialPackageJson)) {
49+
const pkg = JSON.parse(fs.readFileSync(potentialPackageJson, 'utf8'));
50+
if (pkg.name === packageName) {
51+
return currentDir;
52+
}
53+
}
54+
currentDir = path.dirname(currentDir);
55+
}
56+
57+
throw new Error(`Could not find package root for ${packageName}`);
58+
}
59+
} catch (error) {
60+
console.error(`Error resolving ${packageName}:`, error.message);
61+
process.exit(1);
62+
}
63+
}
64+
65+
/**
66+
* Recursively copy a directory
67+
* @param {string} src - Source directory
68+
* @param {string} dest - Destination directory
69+
*/
70+
function copyDir(src, dest) {
71+
// Create destination directory
72+
fs.mkdirSync(dest, { recursive: true });
73+
74+
// Read all entries in source directory
75+
const entries = fs.readdirSync(src, { withFileTypes: true });
76+
77+
for (const entry of entries) {
78+
const srcPath = path.join(src, entry.name);
79+
const destPath = path.join(dest, entry.name);
80+
81+
if (entry.isDirectory()) {
82+
copyDir(srcPath, destPath);
83+
} else {
84+
fs.copyFileSync(srcPath, destPath);
85+
}
86+
}
87+
}
88+
89+
/**
90+
* Main bundling logic
91+
*/
92+
function bundleDependencies() {
93+
console.log('Bundling dependencies for extension packaging...\n');
94+
95+
// Create out/node_modules directory if it doesn't exist
96+
fs.mkdirSync(OUT_DIR, { recursive: true });
97+
98+
// Bundle each dependency
99+
for (const dep of DEPS_TO_BUNDLE) {
100+
console.log(`Bundling ${dep}...`);
101+
102+
const sourcePath = resolvePackagePath(dep);
103+
const destPath = path.join(OUT_DIR, dep);
104+
105+
// Copy the package
106+
copyDir(sourcePath, destPath);
107+
108+
console.log(` ✓ Copied from ${sourcePath}`);
109+
console.log(` ✓ To ${destPath}\n`);
110+
}
111+
112+
console.log(`✓ Successfully bundled ${DEPS_TO_BUNDLE.length} dependencies`);
113+
}
114+
115+
// Run the bundling
116+
try {
117+
bundleDependencies();
118+
} catch (error) {
119+
console.error('Error bundling dependencies:', error);
120+
process.exit(1);
121+
}

src/test/unit/TagParser.test.ts

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
import { expect } from 'chai';
2+
import * as sinon from 'sinon';
3+
import { TagParser, NoteMetadata, NoteTag } from '../../tagging/TagParser';
4+
5+
describe('TagParser', () => {
6+
let clock: sinon.SinonFakeTimers;
7+
8+
beforeEach(() => {
9+
// Use a fixed date for deterministic testing: 2024-01-15T12:00:00.000Z
10+
clock = sinon.useFakeTimers(new Date('2024-01-15T12:00:00.000Z'));
11+
});
12+
13+
afterEach(() => {
14+
clock.restore();
15+
});
16+
describe('hasFrontmatter', () => {
17+
it('should return true for content with YAML frontmatter', () => {
18+
const content = `---
19+
tags: [test, example]
20+
---
21+
22+
Note content`;
23+
24+
const hasFrontmatter = TagParser.hasFrontmatter(content);
25+
expect(hasFrontmatter).to.be.true;
26+
});
27+
28+
it('should return false for content without frontmatter', () => {
29+
const content = 'Just a regular note without frontmatter';
30+
31+
const hasFrontmatter = TagParser.hasFrontmatter(content);
32+
expect(hasFrontmatter).to.be.false;
33+
});
34+
35+
it('should return false for empty content', () => {
36+
const content = '';
37+
38+
const hasFrontmatter = TagParser.hasFrontmatter(content);
39+
expect(hasFrontmatter).to.be.false;
40+
});
41+
});
42+
43+
describe('parseTags', () => {
44+
it('should parse tags from YAML frontmatter array', () => {
45+
const content = `---
46+
tags: [bug, urgent, feature]
47+
---
48+
49+
Note content`;
50+
51+
const metadata = TagParser.parseTags(content);
52+
expect(metadata.tags).to.have.lengthOf(3);
53+
expect(metadata.tags.map(t => t.name)).to.have.members(['bug', 'urgent', 'feature']);
54+
expect(metadata.tags[0].source).to.equal('manual');
55+
expect(metadata.tags[0].confidence).to.equal(1.0);
56+
});
57+
58+
it('should parse tags from YAML frontmatter list format', () => {
59+
const content = `---
60+
tags:
61+
- project
62+
- frontend
63+
- typescript
64+
---
65+
66+
Note content`;
67+
68+
const metadata = TagParser.parseTags(content);
69+
expect(metadata.tags).to.have.lengthOf(3);
70+
expect(metadata.tags.map(t => t.name)).to.have.members(['project', 'frontend', 'typescript']);
71+
});
72+
73+
it('should return empty array for content without tags', () => {
74+
const content = `---
75+
title: My Note
76+
---
77+
78+
Content`;
79+
80+
const metadata = TagParser.parseTags(content);
81+
expect(metadata.tags).to.have.lengthOf(0);
82+
});
83+
84+
it('should return empty array for content without frontmatter', () => {
85+
const content = 'Regular note content';
86+
87+
const metadata = TagParser.parseTags(content);
88+
expect(metadata.tags).to.have.lengthOf(0);
89+
});
90+
91+
it('should parse tagged-at timestamp when present', () => {
92+
const content = `---
93+
tags: [bug, feature]
94+
tagged-at: 2025-01-01T00:00:00.000Z
95+
---
96+
97+
Content`;
98+
99+
const metadata = TagParser.parseTags(content);
100+
expect(metadata.tags).to.have.lengthOf(2);
101+
expect(metadata.lastTagged).to.be.instanceOf(Date);
102+
expect(metadata.lastTagged?.toISOString()).to.equal('2025-01-01T00:00:00.000Z');
103+
});
104+
105+
it('should parse old string format tags', () => {
106+
const content = `---
107+
tags: bug, feature, urgent
108+
---
109+
110+
Content`;
111+
112+
const metadata = TagParser.parseTags(content);
113+
expect(metadata.tags).to.have.lengthOf(3);
114+
expect(metadata.tags.map(t => t.name)).to.have.members(['bug', 'feature', 'urgent']);
115+
});
116+
});
117+
118+
describe('writeTags', () => {
119+
it('should add frontmatter with tags to content without frontmatter', () => {
120+
const content = 'Note content';
121+
const metadata: NoteMetadata = {
122+
tags: [
123+
{ name: 'bug', confidence: 1.0, source: 'manual', addedAt: new Date() },
124+
{ name: 'feature', confidence: 1.0, source: 'manual', addedAt: new Date() }
125+
]
126+
};
127+
128+
const result = TagParser.writeTags(content, metadata);
129+
expect(result).to.include('---');
130+
expect(result).to.include('tags: [bug, feature]');
131+
expect(result).to.include('Note content');
132+
});
133+
134+
it('should update existing frontmatter with new tags', () => {
135+
const content = `---
136+
title: My Note
137+
---
138+
139+
Content`;
140+
const metadata: NoteMetadata = {
141+
tags: [
142+
{ name: 'urgent', confidence: 1.0, source: 'manual', addedAt: new Date() },
143+
{ name: 'todo', confidence: 1.0, source: 'manual', addedAt: new Date() }
144+
]
145+
};
146+
147+
const result = TagParser.writeTags(content, metadata);
148+
expect(result).to.include('title: My Note');
149+
expect(result).to.include('tags: [urgent, todo]');
150+
});
151+
152+
it('should preserve existing frontmatter fields', () => {
153+
const content = `---
154+
title: My Note
155+
author: John
156+
---
157+
158+
Content`;
159+
const metadata: NoteMetadata = {
160+
tags: [
161+
{ name: 'new', confidence: 1.0, source: 'manual', addedAt: new Date() }
162+
]
163+
};
164+
165+
const result = TagParser.writeTags(content, metadata);
166+
expect(result).to.include('title: My Note');
167+
expect(result).to.include('author: John');
168+
expect(result).to.include('tags: [new]');
169+
});
170+
171+
it('should add tagged-at timestamp when tags are added', () => {
172+
const content = 'Content';
173+
const metadata: NoteMetadata = {
174+
tags: [
175+
{ name: 'test', confidence: 1.0, source: 'manual', addedAt: new Date() }
176+
]
177+
};
178+
179+
const result = TagParser.writeTags(content, metadata);
180+
// YAML may quote the timestamp string, so check for both formats
181+
const hasTimestamp = result.includes('tagged-at: 2024-01-15T12:00:00.000Z') ||
182+
result.includes("tagged-at: '2024-01-15T12:00:00.000Z'");
183+
expect(hasTimestamp).to.be.true;
184+
});
185+
186+
it('should remove tags field when metadata has empty tags', () => {
187+
const content = `---
188+
tags: [bug, feature]
189+
title: My Note
190+
---
191+
192+
Content`;
193+
const metadata: NoteMetadata = {
194+
tags: []
195+
};
196+
197+
const result = TagParser.writeTags(content, metadata);
198+
expect(result).to.not.include('tags:');
199+
expect(result).to.include('title: My Note');
200+
});
201+
});
202+
203+
describe('removeTags', () => {
204+
it('should remove all tags from frontmatter', () => {
205+
const content = `---
206+
tags: [bug, feature]
207+
title: My Note
208+
---
209+
210+
Content`;
211+
212+
const result = TagParser.removeTags(content);
213+
expect(result).to.include('title: My Note');
214+
expect(result).to.not.include('tags:');
215+
});
216+
217+
it('should handle content without tags gracefully', () => {
218+
const content = `---
219+
title: My Note
220+
---
221+
222+
Content`;
223+
224+
const result = TagParser.removeTags(content);
225+
expect(result).to.include('title: My Note');
226+
});
227+
228+
it('should handle content without frontmatter gracefully', () => {
229+
const content = 'Just content';
230+
231+
const result = TagParser.removeTags(content);
232+
expect(result).to.equal('Just content');
233+
});
234+
});
235+
});

0 commit comments

Comments
 (0)