Skip to content

Commit

Permalink
fix: Include pinned files in the content considered by vector terms
Browse files Browse the repository at this point in the history
Fixes #2199
  • Loading branch information
kgilpin authored and dustinbyrne committed Jan 22, 2025
1 parent ab1b0b6 commit 355d931
Show file tree
Hide file tree
Showing 20 changed files with 288 additions and 111 deletions.
File renamed without changes.
70 changes: 70 additions & 0 deletions architecture/navie/file-content-fetcher.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
File content fetcher is used to detect the locations of files that are mentioned in the chat, and to
fetch the content of those files.

### Sequence Diagram

```mermaid
sequenceDiagram
participant GenerateAgent as GenerateAgent
participant TestAgent as TestAgent
participant ClientRequest as ClientRequest
participant ChatHistory as ChatHistory
participant FileContentFetcher as FileContentFetcher
participant FileChangeExtractorService as FileChangeExtractorService
participant ContextService as ContextService
alt Invocation by GenerateAgent
GenerateAgent->>FileContentFetcher: applyFileContext(options, options.chatHistory)
end
alt Invocation by TestAgent
TestAgent->>FileContentFetcher: applyFileContext(options, options.chatHistory)
end
activate FileContentFetcher
FileContentFetcher->>FileChangeExtractorService: listFiles(clientRequest, chatHistory)
activate FileChangeExtractorService
FileChangeExtractorService-->>FileContentFetcher: fileNames
deactivate FileChangeExtractorService
alt FileNames Found
FileContentFetcher->>ContextService: locationContext(fileNames)
activate ContextService
ContextService-->>FileContentFetcher: Context updated
deactivate ContextService
else No FileNames
FileContentFetcher-->>ClientRequest: return undefined
end
deactivate FileContentFetcher
```

### Class Map

```mermaid
classDiagram
class FileContentFetcher {
+applyFileContext(clientRequest: ClientRequest, chatHistory: ChatHistory | undefined): void
}
class FileChangeExtractorService {
+listFiles(clientRequest: ClientRequest, chatHistory: ChatHistory | undefined): string[]
}
class ContextService {
+locationContext(fileNames: string[]): void
}
class GenerateAgent {
+applyFileContext(options: AgentOptions, chatHistory: ChatHistory | undefined): void
}
class TestAgent {
+applyFileContext(options: AgentOptions, chatHistory: ChatHistory | undefined): void
}
GenerateAgent --> FileContentFetcher : use
TestAgent --> FileContentFetcher : use
FileContentFetcher --> FileChangeExtractorService : relies on
FileContentFetcher --> ContextService : interacts with
```
14 changes: 9 additions & 5 deletions packages/navie/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,25 @@ export class AgentOptions {
return this.projectInfo.some((info) => info.appmapStats && info.appmapStats?.numAppMaps > 0);
}

get contextLocations(): string[] {
get pinnedFileLocations(): string[] {
return this.codeSelection && typeof this.codeSelection !== 'string'
? this.codeSelection.filter(UserContext.hasLocation).map((cs) => cs.location)
: [];
}

/**
* Configure context filters to fetch the content that's relevant to the current user options,
* including /include and /exclude options and pinned file locations.
*/
buildContextFilters(): ContextV2.ContextFilters {
const filters: ContextV2.ContextFilters = {};
if (this.contextLabels) filters.labels = this.contextLabels;
this.userOptions.populateContextFilters(filters);
const contextLocations = this.contextLocations;
if (contextLocations.length > 0) {
filters.locations = contextLocations;
const pinnedFileLocations = this.pinnedFileLocations;
if (pinnedFileLocations.length > 0) {
filters.locations = pinnedFileLocations;
filters.exclude ||= [];
filters.exclude.push(...contextLocations);
filters.exclude.push(...pinnedFileLocations);
}
return filters;
}
Expand Down
1 change: 1 addition & 0 deletions packages/navie/src/agents/explain-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ export default class ExplainAgent implements Agent {
);

await this.contextService.locationContextFromOptions(options);

if (
hasLabel(options.contextLabels, ContextV2.ContextLabelName.Overview)?.weight !==
ContextV2.ContextLabelWeight.High
Expand Down
7 changes: 6 additions & 1 deletion packages/navie/src/agents/gatherer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { debug as makeDebug } from 'node:util';

import {
ContextItemEvent,
ContextItemRequestor,
type InteractionEvent,
PromptInteractionEvent,
} from '../interaction-history';
Expand Down Expand Up @@ -64,7 +65,11 @@ export default class Gatherer {

let response = '';
if (locations.length > 0 || terms.length > 0) {
for (const event of await this.context.searchContextWithLocations(terms, locations)) {
for (const event of await this.context.searchContextWithLocations(
ContextItemRequestor.Gatherer,
terms,
locations
)) {
const location = event.location?.startsWith(event.directory ?? '')
? event.location
: [event.directory, event.location].filter(Boolean).join('/');
Expand Down
34 changes: 33 additions & 1 deletion packages/navie/src/interaction-history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,19 @@ export class HelpLookupEvent extends InteractionEvent {
}
}

export enum ContextItemRequestor {
Gatherer = 'gatherer',
Memory = 'memory',
Mentions = 'mentions',
PinnedFile = 'pinnedFile',
ProjectInfo = 'projectInfo',
Terms = 'terms',
}

export class ContextItemEvent extends InteractionEvent {
constructor(
public promptType: PromptType,
public requestor: ContextItemRequestor,
public content: string,
public location?: string | undefined,
public directory?: string | undefined
Expand Down Expand Up @@ -286,16 +296,24 @@ export class TechStackEvent extends InteractionEvent {
class InteractionHistory extends EventEmitter {
public readonly events: InteractionEvent[] = [];

private acceptPinnedFileContext = true;

// eslint-disable-next-line class-methods-use-this
log(message: string) {
console.log(message);
console.warn(message);
}

addEvent(event: InteractionEvent) {
if (!this.validateEvent(event)) return;

this.emit('event', event);
this.events.push(event);
}

stopAcceptingPinnedFileContext() {
this.acceptPinnedFileContext = false;
}

clear() {
this.events.splice(0, this.events.length);
}
Expand Down Expand Up @@ -328,6 +346,20 @@ class InteractionHistory extends EventEmitter {

return state;
}

protected validateEvent(event: InteractionEvent) {
if (event instanceof ContextItemEvent) {
if (event.requestor === ContextItemRequestor.PinnedFile && !this.acceptPinnedFileContext) {
this.log(
'WARNING Unexpected pinned file context item; no further pinned file context is expected.'
);
// This is a warning only. We don't want to stop the event from being added.
return true;
}
}

return true;
}
}

interface InteractionHistory {
Expand Down
10 changes: 8 additions & 2 deletions packages/navie/src/services/apply-context-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { warn } from 'console';
import { ContextV2 } from '../context';
import InteractionHistory, {
ContextItemEvent,
ContextItemRequestor,
PromptInteractionEvent,
} from '../interaction-history';
import { PromptType, buildPromptDescriptor } from '../prompt';
Expand All @@ -13,6 +14,7 @@ export default class ApplyContextService {
constructor(public readonly interactionHistory: InteractionHistory) {}

applyContext(
requestor: ContextItemRequestor,
context: ContextV2.ContextResponse | undefined,
help: HelpResponse,
characterLimit: number,
Expand Down Expand Up @@ -44,7 +46,7 @@ export default class ApplyContextService {
const charsRemaining = characterLimit - charsApplied;

for (const item of appliedContextItems) {
const event = eventOfContextItem(item);
const event = eventOfContextItem(requestor, item);
if (event) this.interactionHistory.addEvent(event);
}

Expand Down Expand Up @@ -102,7 +104,10 @@ export default class ApplyContextService {
}
}

export function eventOfContextItem(item: ContextV2.ContextItem): undefined | ContextItemEvent {
export function eventOfContextItem(
requestor: ContextItemRequestor,
item: ContextV2.ContextItem
): undefined | ContextItemEvent {
let promptType: PromptType | undefined;
switch (item.type) {
case ContextV2.ContextItemType.SequenceDiagram:
Expand All @@ -125,6 +130,7 @@ export function eventOfContextItem(item: ContextV2.ContextItem): undefined | Con
const isFile = ContextV2.isFileContextItem(item);
return new ContextItemEvent(
promptType,
requestor,
item.content,
isFile ? item.location : undefined,
isFile ? item.directory : undefined
Expand Down
34 changes: 28 additions & 6 deletions packages/navie/src/services/context-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import transformSearchTerms from '../lib/transform-search-terms';
import LookupContextService from './lookup-context-service';
import VectorTermsService from './vector-terms-service';
import { ContextV2 } from '../context';
import InteractionHistory, { ContextItemEvent } from '../interaction-history';
import InteractionHistory, { ContextItemEvent, ContextItemRequestor } from '../interaction-history';
import ApplyContextService, { eventOfContextItem } from './apply-context-service';

export default class ContextService {
Expand All @@ -15,6 +15,9 @@ export default class ContextService {
private applyContextService: ApplyContextService
) {}

/**
* Populates the interaction history with context obtained by searching the project.
*/
async searchContext(
options: AgentOptions,
tokensAvailable: () => number,
Expand All @@ -25,6 +28,18 @@ export default class ContextService {
if (contextEnabled) {
this.history.log('[context-service] Searching for context');

this.history.stopAcceptingPinnedFileContext();

const aggregateQuestion = [...options.aggregateQuestion];
// Add content obtained from pinned files
for (const event of this.history.events) {
if (!(event instanceof ContextItemEvent)) continue;

if (!(event.requestor === ContextItemRequestor.PinnedFile)) continue;

aggregateQuestion.push(event.content);
}

const searchTerms = await transformSearchTerms(
termsEnabled,
options.aggregateQuestion,
Expand All @@ -47,7 +62,13 @@ export default class ContextService {
}
}

async locationContext(fileNames: string[]): Promise<ContextItemEvent[]> {
/**
* Populates the interaction history with file contents of the provided file names.
*/
async locationContext(
requestor: ContextItemRequestor,
fileNames: string[]
): Promise<ContextItemEvent[]> {
if (!fileNames || fileNames.length === 0) {
this.history.log('[context-service] No file names provided for location context');
return [];
Expand All @@ -66,8 +87,9 @@ export default class ContextService {
let charsAdded = 0;
const events: ContextItemEvent[] = [];
for (const item of context) {
const contextItem = eventOfContextItem(item);
const contextItem = eventOfContextItem(requestor, item);
if (!contextItem) continue;

charsAdded += contextItem.content.length;
events.push(contextItem);
this.history.addEvent(contextItem);
Expand All @@ -77,6 +99,7 @@ export default class ContextService {
}

async searchContextWithLocations(
requestor: ContextItemRequestor,
searchTerms: string[],
fileNames: string[]
): Promise<ContextItemEvent[]> {
Expand All @@ -88,7 +111,7 @@ export default class ContextService {
let charsAdded = 0;
const events: ContextItemEvent[] = [];
for (const item of ContextService.guardContextType(context)) {
const contextItem = eventOfContextItem(item);
const contextItem = eventOfContextItem(requestor, item);
if (!contextItem) continue;
charsAdded += contextItem.content.length;
events.push(contextItem);
Expand All @@ -102,8 +125,7 @@ export default class ContextService {
const locations = options.buildContextFilters().locations ?? [];
// Also list project directories
locations.unshift(':0');
console.log(locations);
await this.locationContext(locations);
await this.locationContext(ContextItemRequestor.PinnedFile, locations);
}

static guardContextType(
Expand Down
6 changes: 5 additions & 1 deletion packages/navie/src/services/file-content-fetcher.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { ContextItemRequestor } from '../interaction-history';
import { ClientRequest, ChatHistory } from '../navie';
import ContextService from './context-service';
import FileChangeExtractorService from './file-change-extractor-service';

/**
* Used to detect file mentions within a chat interaction and to fetch their content using helper services.
*/
export default class FileContentFetcher {
constructor(
private fileChangeExtractor: FileChangeExtractorService,
Expand All @@ -17,6 +21,6 @@ export default class FileContentFetcher {
return undefined;
}

await this.contextService.locationContext(fileNames);
await this.contextService.locationContext(ContextItemRequestor.Mentions, fileNames);
}
}
13 changes: 11 additions & 2 deletions packages/navie/src/services/lookup-context-service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { warn } from 'console';
import InteractionHistory, { ContextLookupEvent, HelpLookupEvent } from '../interaction-history';
import InteractionHistory, {
ContextItemRequestor,
ContextLookupEvent,
HelpLookupEvent,
} from '../interaction-history';
import { ContextV2 } from '../context';
import { CHARACTERS_PER_TOKEN } from '../message';
import ApplyContextService from './apply-context-service';
Expand Down Expand Up @@ -67,6 +71,11 @@ export default class LookupContextService {
) {
applyContextService.addSystemPrompts(context, help);

applyContextService.applyContext(context, help, tokenCount * CHARACTERS_PER_TOKEN);
applyContextService.applyContext(
ContextItemRequestor.Terms,
context,
help,
tokenCount * CHARACTERS_PER_TOKEN
);
}
}
Loading

0 comments on commit 355d931

Please sign in to comment.