Skip to content

Commit cae623f

Browse files
committed
feat: Observe command can now suggest creating new tests
Other improvements: - enhance UserOptions constructor to accept both Map and object formats; - add clone method to InteractionHistory; - implement closingTags function for unclosed XML tags; - introduce replaceStream for async string replacement in streams; - update ExplainCommand to handle optional classifierService.
1 parent 6fd9d71 commit cae623f

File tree

8 files changed

+281
-33
lines changed

8 files changed

+281
-33
lines changed

packages/navie/src/commands/explain-command.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ export default class ExplainCommand implements Command {
3434
private readonly options: ExplainOptions,
3535
public readonly interactionHistory: InteractionHistory,
3636
private readonly completionService: CompletionService,
37-
private readonly classifierService: ClassificationService,
38-
private readonly agentSelectionService: AgentSelectionService,
37+
private readonly classifierService: ClassificationService | undefined,
38+
private readonly agentSelectionService: Pick<
39+
AgentSelectionService,
40+
'selectAgent' | 'contextService'
41+
>,
3942
private readonly codeSelectionService: CodeSelectionService,
4043
private readonly projectInfoService: ProjectInfoService,
4144
private readonly memoryService: MemoryService
@@ -52,7 +55,7 @@ export default class ExplainCommand implements Command {
5255
let contextLabelsFn: Promise<ContextV2.ContextLabel[]> | undefined;
5356

5457
performance.mark('classifyStart');
55-
if (classifyEnabled)
58+
if (classifyEnabled && this.classifierService)
5659
contextLabelsFn = this.classifierService
5760
.classifyQuestion(baseQuestion, chatHistory)
5861
.catch((err) => {

packages/navie/src/commands/observe-command.ts

Lines changed: 120 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,63 @@
11
import { z } from 'zod';
22

3+
import { AgentMode } from '../agent';
4+
import TestAgent from '../agents/test-agent';
35
import type Command from '../command';
46
import type { CommandRequest } from '../command';
5-
import CompletionService from '../services/completion-service';
6-
import LookupContextService from '../services/lookup-context-service';
7-
import VectorTermsService from '../services/vector-terms-service';
87
import { ContextV2 } from '../context';
9-
import { ExplainOptions } from './explain-command';
10-
import Message from '../message';
118
import InteractionHistory, {
129
CompletionEvent,
1310
PromptInteractionEvent,
1411
} from '../interaction-history';
12+
import closingTags from '../lib/closing-tags';
1513
import { UserOptions } from '../lib/parse-options';
14+
import replaceStream from '../lib/replace-stream';
15+
import Message from '../message';
16+
import ApplyContextService from '../services/apply-context-service';
17+
import CodeSelectionService from '../services/code-selection-service';
18+
import CompletionService from '../services/completion-service';
19+
import ContextService from '../services/context-service';
20+
import FileChangeExtractorService from '../services/file-change-extractor-service';
21+
import LookupContextService from '../services/lookup-context-service';
22+
import { NaiveMemoryService } from '../services/memory-service';
1623
import ProjectInfoService from '../services/project-info-service';
24+
import VectorTermsService from '../services/vector-terms-service';
25+
26+
import ExplainCommand, { ExplainOptions } from './explain-command';
1727

1828
const RelevantTest = z.object({
1929
relevantTest: z
2030
.object({
21-
name: z.string().optional().describe('The name of the test case, if known'),
22-
path: z.string().describe('The file path of the test file'),
31+
name: z
32+
.string()
33+
.optional()
34+
.describe('The name of the test case, if known')
35+
.nullable()
36+
.transform((value) => (value === null ? undefined : value)),
37+
path: z
38+
.string()
39+
.describe('The file path of the test file')
40+
.nullable()
41+
.transform((value) => (value === null ? undefined : value)),
2342
language: z
2443
.enum(['ruby', 'python', 'java', 'javascript', 'other'])
2544
.describe('The programming language of the test file'),
26-
framework: z.string().optional().describe('The test framework used'),
45+
framework: z
46+
.string()
47+
.optional()
48+
.describe('The test framework used')
49+
.nullable()
50+
.transform((value) => (value === null ? undefined : value)),
2751
})
2852
.optional()
53+
.nullable()
2954
.describe('A descriptor of the most relevant test to the requested behavior'),
55+
suggestedTest: z
56+
.string()
57+
.optional()
58+
.nullable()
59+
.transform((value) => (value === null ? undefined : value))
60+
.describe('A suggested test case to write, if no relevant test is found'),
3061
installCommands: z
3162
.array(
3263
z.object({
@@ -126,6 +157,9 @@ export default class ObserveCommand implements Command {
126157
{
127158
role: 'system',
128159
content: `Given the following code snippets, identify the single most relevant test to the user request.
160+
If no test seems relevant, suggest a test case to write instead. Do not provide test case code, just describe it in detail.
161+
The test case should be relevant to the user request, and ideally, it should be written in the same language as the code snippets provided.
162+
Do suggest a name and path for the test case and take it into account when generating the test run instructions.
129163
130164
${projectLanguageDirective}
131165
@@ -156,24 +190,83 @@ ${context.join('\n')}
156190
return result;
157191
}
158192

193+
private async *suggestTestCase(
194+
suggestedTest: string,
195+
userOptions: UserOptions,
196+
history: InteractionHistory
197+
): AsyncIterable<string> {
198+
const applyContextService = new ApplyContextService(history);
199+
const contextService = new ContextService(
200+
history,
201+
this.vectorTermsService,
202+
this.lookupContextService,
203+
applyContextService
204+
);
205+
const testAgent = new TestAgent(
206+
history,
207+
contextService,
208+
new FileChangeExtractorService(history, this.completionService)
209+
);
210+
// create an ExplainCommand
211+
const explainCommand = new ExplainCommand(
212+
this.options,
213+
history,
214+
this.completionService,
215+
undefined,
216+
{
217+
selectAgent: () => ({
218+
agentMode: AgentMode.Test,
219+
agent: testAgent,
220+
question: suggestedTest,
221+
}),
222+
contextService,
223+
},
224+
new CodeSelectionService(history),
225+
this.projectInfoService,
226+
NaiveMemoryService
227+
);
228+
229+
// call the explainCommand with the suggested test
230+
const commandRequest: CommandRequest = {
231+
question: suggestedTest + '\n\nOnly include code, no explanations.',
232+
userOptions: new UserOptions({
233+
...userOptions,
234+
format: 'xml',
235+
classify: false,
236+
tokenlimit: String(userOptions.numberValue('tokenlimit') || this.options.tokenLimit),
237+
}),
238+
};
239+
240+
const explainCommandResponse: string[] = [];
241+
yield '\n';
242+
for await (const token of explainCommand.execute(commandRequest)) {
243+
yield token;
244+
explainCommandResponse.push(token);
245+
}
246+
247+
yield closingTags(explainCommandResponse.join('').trim());
248+
249+
yield '\n';
250+
}
251+
159252
async *execute({ question: userRequest, userOptions }: CommandRequest): AsyncIterable<string> {
160253
const vectorTerms = await this.vectorTermsService.suggestTerms(userRequest);
161254
const tokenLimit = userOptions.numberValue('tokenlimit') || this.options.tokenLimit;
162255
const testSnippets = await this.getTestSnippets(vectorTerms, tokenLimit);
163256
const result = await this.getMostRelevantTest(userRequest, userOptions, testSnippets);
164-
const { relevantTest, installCommands, testCommands } = result || {};
165-
if (!relevantTest) {
166-
yield 'Sorry, I could not find any relevant tests to record.';
167-
return;
168-
}
257+
const { relevantTest, installCommands, testCommands, suggestedTest } = result || {};
258+
259+
const history = this.interactionHistory.clone();
169260

170-
if (relevantTest.language === 'other') {
261+
if (relevantTest?.language === 'other') {
171262
yield `I found a relevant test at \`${relevantTest.path}\`, but I'm unable to help you record it at this time. This language does not appear to be supported.`;
172263
return;
173264
}
174265

175266
const helpDocs = await this.lookupContextService.lookupHelp(
176-
['record', 'agent', 'tests', relevantTest.framework].filter(Boolean) as string[],
267+
['record', 'agent', 'tests', relevantTest?.language, relevantTest?.framework].filter(
268+
Boolean
269+
) as string[],
177270
tokenLimit
178271
);
179272

@@ -191,11 +284,15 @@ ${userRequest}
191284
},
192285
{
193286
role: 'assistant',
194-
content: `Based on the request, the most relevant test case is:
287+
content:
288+
(relevantTest?.path
289+
? `Based on the request, the most relevant test case is:
195290
${relevantTest.name ? `**Name:** \`${relevantTest.name}\`` : ''}
196291
${relevantTest.framework ? `**Framework:** \`${relevantTest.framework}\`` : ''}
197292
${relevantTest.language ? `**Language:** \`${relevantTest.language}\`` : ''}
198-
**Path:** \`${relevantTest.path}\`
293+
**Path:** \`${relevantTest.path}\``
294+
: `Based on the request, a ${relevantTest?.language} ${relevantTest?.framework} test case needs to be created first:\n${suggestedTest}\n\n`) +
295+
`
199296
200297
${
201298
installCommands?.length
@@ -227,11 +324,11 @@ ${helpDocs
227324
},
228325
{
229326
role: 'user',
230-
content: `Restate the information you've provided to me, in standalone format, as a step by step guide outlining the steps required to record the single test case that you've identified.
327+
content: `Restate the information you've provided to me, in standalone format, as a step by step guide outlining the steps required to record the single test case that you've identified or suggested creating.
231328
If possible, include the terminal command needed to run the test. Only specify test patterns that are guaranteed to match based on previous context. For example, do not include file ranges not supported by the test runner.
232329
In your response, please include the following:
233-
- The name of the test case (if known)
234-
- The path to the test file
330+
- If an existing test was found, indicate the test case name and path
331+
- Otherwise, steps and suggested location to create it (don't generate code itself, instead use <generated-test-case /> placeholder — DO NOT surround it with code fences)
235332
- Any steps and terminal commands required to install the AppMap recording agent
236333
- Any steps and terminal commands required to run the specific test case
237334
@@ -250,9 +347,9 @@ Do not include:
250347
);
251348
const completion = this.completionService.complete(messages, { temperature });
252349

253-
for await (const token of completion) {
254-
yield token;
255-
}
350+
yield* replaceStream(completion, '<generated-test-case />', () =>
351+
this.suggestTestCase(suggestedTest ?? '', userOptions, history)
352+
);
256353
}
257354
}
258355

packages/navie/src/interaction-history.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,21 @@ class InteractionHistory extends EventEmitter {
316316
this.events.push(event);
317317
}
318318

319+
/**
320+
* Clone the interaction history.
321+
* This is useful for creating a copy of the interaction history
322+
* that can be modified without affecting the original.
323+
* @note this is a shallow copy, so the events are not cloned.
324+
* This means that modifying the events in the clone will also modify the events in the original.
325+
* @returns a shallow copy of the interaction history
326+
*/
327+
clone(): InteractionHistory {
328+
const clone = new InteractionHistory();
329+
clone.events.push(...this.events);
330+
clone.acceptPinnedFileContext = this.acceptPinnedFileContext;
331+
return clone;
332+
}
333+
319334
stopAcceptingPinnedFileContext() {
320335
this.acceptPinnedFileContext = false;
321336
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/** Scan the text for any unclosed XML tags and returns the closing tags to be appended. */
2+
export default function closingTags(text: string): string {
3+
// Stack to keep track of open tags
4+
const openTags: string[] = [];
5+
6+
// Match opening and self-closing tags
7+
const tagRegex = /<(\/)?([a-zA-Z]+)([^>]*?)(\/)?>/g;
8+
9+
for (const match of text.matchAll(tagRegex)) {
10+
const [, close, tagName, attributes, selfClosing] = match;
11+
12+
// Skip self-closing tags
13+
if (selfClosing) continue;
14+
15+
// Check if this is a closing tag
16+
if (close || attributes.trim().startsWith('/')) {
17+
// Found a closing tag, remove the matching opening tag if it exists
18+
const lastOpenTag = openTags[openTags.length - 1];
19+
if (lastOpenTag === tagName) {
20+
openTags.pop();
21+
}
22+
} else {
23+
// Found an opening tag, push it onto the stack
24+
openTags.push(tagName);
25+
}
26+
}
27+
28+
// Generate closing tags for any remaining open tags in reverse order
29+
return openTags
30+
.reverse()
31+
.map((tag) => `</${tag}>`)
32+
.join('');
33+
}

packages/navie/src/lib/parse-options.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
11
import { ContextV2 } from '../context';
22

33
export class UserOptions {
4-
constructor(private options: Map<string, string | boolean>) {}
4+
private options: Map<string, string | boolean>;
5+
constructor(options: Map<string, string | boolean> | Record<string, string | boolean>) {
6+
this.options = new Map<string, string | boolean>();
7+
if (options instanceof Map) {
8+
this.options = options;
9+
} else {
10+
Object.entries(options).forEach(([key, value]) => {
11+
this.options.set(key.toLowerCase(), value);
12+
});
13+
}
14+
}
15+
16+
[Symbol.iterator](): IterableIterator<[string, string | boolean]> {
17+
return this.options.entries();
18+
}
519

620
has(key: string): boolean {
721
return this.options.has(key);
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import splitOn from './split-on';
2+
3+
/**
4+
* Replace a needle in a stream with a replacement function.
5+
* The replacement function is called with the found string and returns an async iterable of strings.
6+
* The replacement function can be async, and will be awaited.
7+
* The replacement function can yield multiple strings.
8+
* The replacement function can be called multiple times if the needle is found multiple times.
9+
* @param source the source stream
10+
* @param needle the string or regex to search
11+
* @param replacement the replacement function
12+
*/
13+
export default async function* replaceStream(
14+
source: AsyncIterable<string>,
15+
needle: string | RegExp,
16+
replacement: (found: string) => AsyncIterable<string>
17+
): AsyncIterable<string> {
18+
let buffer = '';
19+
for await (const chunk of source) {
20+
buffer += chunk;
21+
while (buffer) {
22+
const [before, found, after] = splitOn(buffer, needle);
23+
yield before;
24+
if (found) {
25+
if (after) {
26+
yield* replacement(found);
27+
} else {
28+
buffer = found;
29+
break;
30+
}
31+
}
32+
buffer = after;
33+
}
34+
}
35+
}

packages/navie/test/commands/observe-command.spec.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,35 @@ describe('ObserveCommand', () => {
107107
expect(lookupContextService.lookupHelp).toBeCalledTimes(1);
108108
});
109109

110-
it('exits early if no test is identified', async () => {
110+
it('suggests a test if no test is identified', async () => {
111111
lookupContextService.lookupContext = jest.fn().mockResolvedValue([]);
112-
completionService.json = jest.fn().mockReturnValueOnce(undefined);
113-
lookupContextService.lookupHelp = jest.fn();
112+
completionService.json = jest.fn().mockReturnValueOnce({
113+
suggestedTest: 'Write a test for the observe command that handles missing tests',
114+
relevantTest: { language: 'javascript' },
115+
});
116+
completionService.complete = jest
117+
.fn()
118+
.mockImplementationOnce(function* () {
119+
yield '1. Create the following test:\n<generated-test-case />\n2. Run the test';
120+
})
121+
.mockImplementation(function* () {
122+
yield 'Generated test case code';
123+
});
124+
114125
const result = await read(
115126
command.execute({
116127
question: 'what?',
117128
userOptions: new UserOptions(new Map()),
118129
})
119130
);
120-
expect(result).toEqual('Sorry, I could not find any relevant tests to record.');
121-
expect(lookupContextService.lookupHelp).not.toBeCalled();
122-
expect(completionService.complete).not.toBeCalled();
131+
expect(result).toMatchInlineSnapshot(`
132+
"1. Create the following test:
133+
134+
Generated test case code
135+
136+
2. Run the test"
137+
`);
138+
expect(completionService.complete).toHaveBeenCalledTimes(4);
123139
});
124140

125141
it('exits early if the language is not supported', async () => {

0 commit comments

Comments
 (0)