mirror of https://github.com/buster-so/buster.git
linting
This commit is contained in:
parent
76d14d281f
commit
320c3f5413
|
@ -17,6 +17,7 @@ jobs:
|
|||
name: Lint
|
||||
runs-on: blacksmith-2vcpu-ubuntu-2404
|
||||
timeout-minutes: 5
|
||||
environment: testing
|
||||
steps:
|
||||
- name: Checkout code
|
||||
uses: actions/checkout@v4
|
||||
|
|
|
@ -98,10 +98,10 @@ export function createAnalystAgent(analystAgentOptions: AnalystAgentOptions) {
|
|||
|
||||
const docsSystemMessage = docsContent
|
||||
? ({
|
||||
role: 'system',
|
||||
content: `<data_catalog_docs>\n${docsContent}\n</data_catalog_docs>`,
|
||||
providerOptions: DEFAULT_ANTHROPIC_OPTIONS,
|
||||
} as ModelMessage)
|
||||
role: 'system',
|
||||
content: `<data_catalog_docs>\n${docsContent}\n</data_catalog_docs>`,
|
||||
providerOptions: DEFAULT_ANTHROPIC_OPTIONS,
|
||||
} as ModelMessage)
|
||||
: null;
|
||||
|
||||
async function stream({ messages }: AnalystStreamOptions) {
|
||||
|
@ -134,19 +134,19 @@ export function createAnalystAgent(analystAgentOptions: AnalystAgentOptions) {
|
|||
// Create analyst instructions system message with proper escaping
|
||||
const analystInstructionsMessage = analystInstructions
|
||||
? ({
|
||||
role: 'system',
|
||||
content: `<organization_instructions>\n${analystInstructions}\n</organization_instructions>`,
|
||||
providerOptions: DEFAULT_ANTHROPIC_OPTIONS,
|
||||
} as ModelMessage)
|
||||
role: 'system',
|
||||
content: `<organization_instructions>\n${analystInstructions}\n</organization_instructions>`,
|
||||
providerOptions: DEFAULT_ANTHROPIC_OPTIONS,
|
||||
} as ModelMessage)
|
||||
: null;
|
||||
|
||||
// Create user personalization system message
|
||||
const userPersonalizationSystemMessage = userPersonalizationMessageContent
|
||||
? ({
|
||||
role: 'system',
|
||||
content: userPersonalizationMessageContent,
|
||||
providerOptions: DEFAULT_ANTHROPIC_OPTIONS,
|
||||
} as ModelMessage)
|
||||
role: 'system',
|
||||
content: userPersonalizationMessageContent,
|
||||
providerOptions: DEFAULT_ANTHROPIC_OPTIONS,
|
||||
} as ModelMessage)
|
||||
: null;
|
||||
|
||||
return wrapTraced(
|
||||
|
|
|
@ -83,10 +83,14 @@ describe('Analyst Agent Instructions', () => {
|
|||
expect(result).toContain('MANDATORY SQL NAMING CONVENTIONS');
|
||||
|
||||
// Ensure table references require full qualification
|
||||
expect(result).toContain('All Table References: MUST be fully qualified: `DATABASE_NAME.SCHEMA_NAME.TABLE_NAME`');
|
||||
expect(result).toContain(
|
||||
'All Table References: MUST be fully qualified: `DATABASE_NAME.SCHEMA_NAME.TABLE_NAME`'
|
||||
);
|
||||
|
||||
// Ensure column references use table aliases (not full qualifiers)
|
||||
expect(result).toContain('All Column References: MUST be qualified with their table alias (e.g., `c.customerid`)');
|
||||
expect(result).toContain(
|
||||
'All Column References: MUST be qualified with their table alias (e.g., `c.customerid`)'
|
||||
);
|
||||
|
||||
// Ensure examples show table alias usage without full qualification
|
||||
expect(result).toContain('c.customerid');
|
||||
|
|
|
@ -151,16 +151,23 @@ describe('Think and Prep Agent Instructions', () => {
|
|||
['investigation', 'investigation'],
|
||||
])('SQL naming conventions in %s mode', (modeName, mode) => {
|
||||
it(`should contain mandatory SQL naming conventions in ${modeName} mode`, () => {
|
||||
const result = getThinkAndPrepAgentSystemPrompt('Test guidance', mode as 'standard' | 'investigation');
|
||||
const result = getThinkAndPrepAgentSystemPrompt(
|
||||
'Test guidance',
|
||||
mode as 'standard' | 'investigation'
|
||||
);
|
||||
|
||||
// Check for MANDATORY SQL NAMING CONVENTIONS section
|
||||
expect(result).toContain('MANDATORY SQL NAMING CONVENTIONS');
|
||||
|
||||
// Ensure table references require full qualification
|
||||
expect(result).toContain('All Table References: MUST be fully qualified: `DATABASE_NAME.SCHEMA_NAME.TABLE_NAME`');
|
||||
expect(result).toContain(
|
||||
'All Table References: MUST be fully qualified: `DATABASE_NAME.SCHEMA_NAME.TABLE_NAME`'
|
||||
);
|
||||
|
||||
// Ensure column references use table aliases (not full qualifiers)
|
||||
expect(result).toContain('All Column References: MUST be qualified with their table alias (e.g., `c.customerid`)');
|
||||
expect(result).toContain(
|
||||
'All Column References: MUST be qualified with their table alias (e.g., `c.customerid`)'
|
||||
);
|
||||
|
||||
// Ensure examples show table alias usage without full qualification
|
||||
expect(result).toContain('c.customerid');
|
||||
|
@ -172,7 +179,10 @@ describe('Think and Prep Agent Instructions', () => {
|
|||
});
|
||||
|
||||
it(`should use column names qualified with table aliases in ${modeName} mode`, () => {
|
||||
const result = getThinkAndPrepAgentSystemPrompt('Test guidance', mode as 'standard' | 'investigation');
|
||||
const result = getThinkAndPrepAgentSystemPrompt(
|
||||
'Test guidance',
|
||||
mode as 'standard' | 'investigation'
|
||||
);
|
||||
|
||||
// Check for the updated description
|
||||
expect(result).toContain('Use column names qualified with table aliases');
|
||||
|
|
|
@ -14,7 +14,7 @@ export const DEFAULT_ANTHROPIC_OPTIONS = {
|
|||
additionalModelRequestFields: {
|
||||
anthropic_beta: ['fine-grained-tool-streaming-2025-05-14'],
|
||||
},
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
export const DEFAULT_OPENAI_OPTIONS = {
|
||||
|
|
|
@ -97,7 +97,7 @@ async function performUpdate({
|
|||
/**
|
||||
* Updates message entries with cache-first approach for streaming.
|
||||
* Cache is the source of truth during streaming, DB is updated for persistence.
|
||||
*
|
||||
*
|
||||
* Updates are queued per messageId to ensure they execute in order.
|
||||
*
|
||||
* Merge logic:
|
||||
|
@ -109,18 +109,18 @@ export async function updateMessageEntries(
|
|||
params: UpdateMessageEntriesParams
|
||||
): Promise<{ success: boolean }> {
|
||||
const { messageId } = params;
|
||||
|
||||
|
||||
// Get the current promise for this messageId, or use a resolved promise as the starting point
|
||||
const currentQueue = updateQueues.get(messageId) ?? Promise.resolve({ success: true });
|
||||
|
||||
|
||||
// Chain the new update to run after the current queue completes
|
||||
const newQueue = currentQueue
|
||||
.then(() => performUpdate(params))
|
||||
.catch(() => performUpdate(params)); // Still try to run even if previous failed
|
||||
|
||||
|
||||
// Update the queue for this messageId
|
||||
updateQueues.set(messageId, newQueue);
|
||||
|
||||
|
||||
// Clean up the queue entry once this update completes
|
||||
newQueue.finally(() => {
|
||||
// Only remove if this is still the current queue
|
||||
|
@ -128,6 +128,6 @@ export async function updateMessageEntries(
|
|||
updateQueues.delete(messageId);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
return newQueue;
|
||||
}
|
||||
|
|
|
@ -32,20 +32,21 @@ type VersionHistoryEntry = {
|
|||
type VersionHistory = Record<string, VersionHistoryEntry>;
|
||||
|
||||
// Simple in-memory queue for each reportId
|
||||
const updateQueues = new Map<string, Promise<{
|
||||
id: string;
|
||||
name: string;
|
||||
content: string;
|
||||
versionHistory: VersionHistory | null;
|
||||
}>>();
|
||||
const updateQueues = new Map<
|
||||
string,
|
||||
Promise<{
|
||||
id: string;
|
||||
name: string;
|
||||
content: string;
|
||||
versionHistory: VersionHistory | null;
|
||||
}>
|
||||
>();
|
||||
|
||||
/**
|
||||
* Internal function that performs the actual update logic.
|
||||
* This is separated so it can be queued.
|
||||
*/
|
||||
async function performUpdate(
|
||||
params: BatchUpdateReportInput
|
||||
): Promise<{
|
||||
async function performUpdate(params: BatchUpdateReportInput): Promise<{
|
||||
id: string;
|
||||
name: string;
|
||||
content: string;
|
||||
|
@ -106,7 +107,7 @@ async function performUpdate(
|
|||
/**
|
||||
* Updates a report with new content, optionally name, and version history in a single operation
|
||||
* This is more efficient than multiple individual updates
|
||||
*
|
||||
*
|
||||
* Updates are queued per reportId to ensure they execute in order.
|
||||
*/
|
||||
export const batchUpdateReport = async (
|
||||
|
@ -118,23 +119,25 @@ export const batchUpdateReport = async (
|
|||
versionHistory: VersionHistory | null;
|
||||
}> => {
|
||||
const { reportId } = params;
|
||||
|
||||
|
||||
// Get the current promise for this reportId, or use a resolved promise as the starting point
|
||||
const currentQueue = updateQueues.get(reportId) ?? Promise.resolve({
|
||||
id: '',
|
||||
name: '',
|
||||
content: '',
|
||||
versionHistory: null
|
||||
});
|
||||
|
||||
const currentQueue =
|
||||
updateQueues.get(reportId) ??
|
||||
Promise.resolve({
|
||||
id: '',
|
||||
name: '',
|
||||
content: '',
|
||||
versionHistory: null,
|
||||
});
|
||||
|
||||
// Chain the new update to run after the current queue completes
|
||||
const newQueue = currentQueue
|
||||
.then(() => performUpdate(params))
|
||||
.catch(() => performUpdate(params)); // Still try to run even if previous failed
|
||||
|
||||
|
||||
// Update the queue for this reportId
|
||||
updateQueues.set(reportId, newQueue);
|
||||
|
||||
|
||||
// Clean up the queue entry once this update completes
|
||||
newQueue.finally(() => {
|
||||
// Only remove if this is still the current queue
|
||||
|
@ -142,6 +145,6 @@ export const batchUpdateReport = async (
|
|||
updateQueues.delete(reportId);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
return newQueue;
|
||||
};
|
||||
|
|
Loading…
Reference in New Issue