fix(paperclip): address review fixes for paperclip integration (#900)
- Add CI job for paperclip integration tests with change detection - Add paperclip to valid release integrations - Validate hindsightApiUrl is set in loadConfig() - Log warnings on recall/retain failures instead of silently swallowing - Remove hardcoded timeout from reflect call - Fix tsconfig module resolution to Node16 - Update tests to pass required hindsightApiUrl
This commit is contained in:
parent
9e2890ba81
commit
7863ffeb49
9 changed files with 61 additions and 18 deletions
35
.github/workflows/test.yml
vendored
35
.github/workflows/test.yml
vendored
|
|
@ -45,6 +45,7 @@ jobs:
|
||||||
integrations-ag2: ${{ steps.filter.outputs.integrations-ag2 }}
|
integrations-ag2: ${{ steps.filter.outputs.integrations-ag2 }}
|
||||||
integrations-hermes: ${{ steps.filter.outputs.integrations-hermes }}
|
integrations-hermes: ${{ steps.filter.outputs.integrations-hermes }}
|
||||||
integrations-llamaindex: ${{ steps.filter.outputs.integrations-llamaindex }}
|
integrations-llamaindex: ${{ steps.filter.outputs.integrations-llamaindex }}
|
||||||
|
integrations-paperclip: ${{ steps.filter.outputs.integrations-paperclip }}
|
||||||
dev: ${{ steps.filter.outputs.dev }}
|
dev: ${{ steps.filter.outputs.dev }}
|
||||||
ci: ${{ steps.filter.outputs.ci }}
|
ci: ${{ steps.filter.outputs.ci }}
|
||||||
# Secrets are available for internal PRs, pull_request_review, and workflow_dispatch.
|
# Secrets are available for internal PRs, pull_request_review, and workflow_dispatch.
|
||||||
|
|
@ -117,6 +118,8 @@ jobs:
|
||||||
- 'hindsight-integrations/hermes/**'
|
- 'hindsight-integrations/hermes/**'
|
||||||
integrations-llamaindex:
|
integrations-llamaindex:
|
||||||
- 'hindsight-integrations/llamaindex/**'
|
- 'hindsight-integrations/llamaindex/**'
|
||||||
|
integrations-paperclip:
|
||||||
|
- 'hindsight-integrations/paperclip/**'
|
||||||
dev:
|
dev:
|
||||||
- 'hindsight-dev/**'
|
- 'hindsight-dev/**'
|
||||||
ci:
|
ci:
|
||||||
|
|
@ -357,6 +360,37 @@ jobs:
|
||||||
working-directory: ./hindsight-integrations/chat
|
working-directory: ./hindsight-integrations/chat
|
||||||
run: npm run build
|
run: npm run build
|
||||||
|
|
||||||
|
test-paperclip-integration:
|
||||||
|
needs: [detect-changes]
|
||||||
|
if: >-
|
||||||
|
github.event_name != 'pull_request_review' &&
|
||||||
|
(github.event_name == 'workflow_dispatch' ||
|
||||||
|
needs.detect-changes.outputs.integrations-paperclip == 'true' ||
|
||||||
|
needs.detect-changes.outputs.ci == 'true')
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v6
|
||||||
|
with:
|
||||||
|
ref: ${{ github.event.pull_request.head.sha || '' }}
|
||||||
|
|
||||||
|
- name: Set up Node.js
|
||||||
|
uses: actions/setup-node@v6
|
||||||
|
with:
|
||||||
|
node-version: '22'
|
||||||
|
|
||||||
|
- name: Install dependencies
|
||||||
|
working-directory: ./hindsight-integrations/paperclip
|
||||||
|
run: npm ci
|
||||||
|
|
||||||
|
- name: Build
|
||||||
|
working-directory: ./hindsight-integrations/paperclip
|
||||||
|
run: npm run build
|
||||||
|
|
||||||
|
- name: Run tests
|
||||||
|
working-directory: ./hindsight-integrations/paperclip
|
||||||
|
run: npm test
|
||||||
|
|
||||||
build-control-plane:
|
build-control-plane:
|
||||||
needs: [detect-changes]
|
needs: [detect-changes]
|
||||||
if: >-
|
if: >-
|
||||||
|
|
@ -2428,6 +2462,7 @@ jobs:
|
||||||
- build-ai-sdk-integration
|
- build-ai-sdk-integration
|
||||||
- test-ai-sdk-integration-deno
|
- test-ai-sdk-integration-deno
|
||||||
- build-chat-integration
|
- build-chat-integration
|
||||||
|
- test-paperclip-integration
|
||||||
- build-control-plane
|
- build-control-plane
|
||||||
- build-docs
|
- build-docs
|
||||||
- test-rust-cli
|
- test-rust-cli
|
||||||
|
|
|
||||||
|
|
@ -78,7 +78,7 @@ export class HindsightClient {
|
||||||
query,
|
query,
|
||||||
budget: options?.budget ?? 'mid',
|
budget: options?.budget ?? 'mid',
|
||||||
max_tokens: options?.maxTokens ?? 1024,
|
max_tokens: options?.maxTokens ?? 1024,
|
||||||
}, 12_000);
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
async retain(
|
async retain(
|
||||||
|
|
|
||||||
|
|
@ -29,7 +29,7 @@ export interface PaperclipMemoryConfig {
|
||||||
}
|
}
|
||||||
|
|
||||||
export function loadConfig(overrides?: Partial<PaperclipMemoryConfig>): PaperclipMemoryConfig {
|
export function loadConfig(overrides?: Partial<PaperclipMemoryConfig>): PaperclipMemoryConfig {
|
||||||
return {
|
const config: PaperclipMemoryConfig = {
|
||||||
hindsightApiUrl: process.env['HINDSIGHT_API_URL'] ?? '',
|
hindsightApiUrl: process.env['HINDSIGHT_API_URL'] ?? '',
|
||||||
hindsightApiToken: process.env['HINDSIGHT_API_TOKEN'],
|
hindsightApiToken: process.env['HINDSIGHT_API_TOKEN'],
|
||||||
bankGranularity: ['company', 'agent'],
|
bankGranularity: ['company', 'agent'],
|
||||||
|
|
@ -40,4 +40,10 @@ export function loadConfig(overrides?: Partial<PaperclipMemoryConfig>): Papercli
|
||||||
timeoutMs: 15_000,
|
timeoutMs: 15_000,
|
||||||
...overrides,
|
...overrides,
|
||||||
};
|
};
|
||||||
|
if (!config.hindsightApiUrl) {
|
||||||
|
throw new Error(
|
||||||
|
'hindsightApiUrl is required — set HINDSIGHT_API_URL or pass hindsightApiUrl to loadConfig()',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return config;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -84,8 +84,8 @@ export function createMemoryMiddleware(config: PaperclipMemoryConfig) {
|
||||||
retain(
|
retain(
|
||||||
{ companyId, agentId, content: output, documentId: runId },
|
{ companyId, agentId, content: output, documentId: runId },
|
||||||
config,
|
config,
|
||||||
).catch(() => {
|
).catch((err) => {
|
||||||
// Graceful degradation
|
console.warn('[hindsight-paperclip] retain failed:', (err as Error).message);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -56,8 +56,8 @@ export async function recall(
|
||||||
maxTokens: config.recallMaxTokens,
|
maxTokens: config.recallMaxTokens,
|
||||||
});
|
});
|
||||||
results = response.results;
|
results = response.results;
|
||||||
} catch {
|
} catch (err) {
|
||||||
// Graceful degradation — memory is enhancement, not requirement
|
console.warn('[hindsight-paperclip] recall failed:', (err as Error).message);
|
||||||
return '';
|
return '';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -52,7 +52,7 @@ export async function retain(
|
||||||
context: config.retainContext,
|
context: config.retainContext,
|
||||||
metadata: { companyId, agentId, ...metadata },
|
metadata: { companyId, agentId, ...metadata },
|
||||||
});
|
});
|
||||||
} catch {
|
} catch (err) {
|
||||||
// Graceful degradation — memory is enhancement, not requirement
|
console.warn('[hindsight-paperclip] retain failed:', (err as Error).message);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -5,38 +5,40 @@ import { loadConfig } from '../src/config.js';
|
||||||
describe('deriveBankId', () => {
|
describe('deriveBankId', () => {
|
||||||
const ctx = { companyId: 'co-123', agentId: 'ag-456' };
|
const ctx = { companyId: 'co-123', agentId: 'ag-456' };
|
||||||
|
|
||||||
|
const baseUrl = 'http://fake:9077';
|
||||||
|
|
||||||
it('default: paperclip::companyId::agentId', () => {
|
it('default: paperclip::companyId::agentId', () => {
|
||||||
const config = loadConfig();
|
const config = loadConfig({ hindsightApiUrl: baseUrl });
|
||||||
expect(deriveBankId(ctx, config)).toBe('paperclip::co-123::ag-456');
|
expect(deriveBankId(ctx, config)).toBe('paperclip::co-123::ag-456');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('company-only granularity', () => {
|
it('company-only granularity', () => {
|
||||||
const config = loadConfig({ bankGranularity: ['company'] });
|
const config = loadConfig({ hindsightApiUrl: baseUrl, bankGranularity: ['company'] });
|
||||||
expect(deriveBankId(ctx, config)).toBe('paperclip::co-123');
|
expect(deriveBankId(ctx, config)).toBe('paperclip::co-123');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('agent-only granularity', () => {
|
it('agent-only granularity', () => {
|
||||||
const config = loadConfig({ bankGranularity: ['agent'] });
|
const config = loadConfig({ hindsightApiUrl: baseUrl, bankGranularity: ['agent'] });
|
||||||
expect(deriveBankId(ctx, config)).toBe('paperclip::ag-456');
|
expect(deriveBankId(ctx, config)).toBe('paperclip::ag-456');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('custom prefix', () => {
|
it('custom prefix', () => {
|
||||||
const config = loadConfig({ bankIdPrefix: 'myapp' });
|
const config = loadConfig({ hindsightApiUrl: baseUrl, bankIdPrefix: 'myapp' });
|
||||||
expect(deriveBankId(ctx, config)).toBe('myapp::co-123::ag-456');
|
expect(deriveBankId(ctx, config)).toBe('myapp::co-123::ag-456');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('empty prefix with default granularity', () => {
|
it('empty prefix with default granularity', () => {
|
||||||
const config = loadConfig({ bankIdPrefix: '' });
|
const config = loadConfig({ hindsightApiUrl: baseUrl, bankIdPrefix: '' });
|
||||||
expect(deriveBankId(ctx, config)).toBe('co-123::ag-456');
|
expect(deriveBankId(ctx, config)).toBe('co-123::ag-456');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('throws when bank ID would be empty', () => {
|
it('throws when bank ID would be empty', () => {
|
||||||
const config = loadConfig({ bankIdPrefix: '', bankGranularity: [] });
|
const config = loadConfig({ hindsightApiUrl: baseUrl, bankIdPrefix: '', bankGranularity: [] });
|
||||||
expect(() => deriveBankId(ctx, config)).toThrow('Bank ID cannot be empty');
|
expect(() => deriveBankId(ctx, config)).toThrow('Bank ID cannot be empty');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('reversed granularity order', () => {
|
it('reversed granularity order', () => {
|
||||||
const config = loadConfig({ bankGranularity: ['agent', 'company'] });
|
const config = loadConfig({ hindsightApiUrl: baseUrl, bankGranularity: ['agent', 'company'] });
|
||||||
expect(deriveBankId(ctx, config)).toBe('paperclip::ag-456::co-123');
|
expect(deriveBankId(ctx, config)).toBe('paperclip::ag-456::co-123');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -1,9 +1,9 @@
|
||||||
{
|
{
|
||||||
"compilerOptions": {
|
"compilerOptions": {
|
||||||
"target": "ES2022",
|
"target": "ES2022",
|
||||||
"module": "ES2022",
|
"module": "Node16",
|
||||||
"lib": ["ES2022"],
|
"lib": ["ES2022"],
|
||||||
"moduleResolution": "node",
|
"moduleResolution": "node16",
|
||||||
"declaration": true,
|
"declaration": true,
|
||||||
"outDir": "./dist",
|
"outDir": "./dist",
|
||||||
"rootDir": "./src",
|
"rootDir": "./src",
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ print_info() { echo -e "${GREEN}[INFO]${NC} $1"; }
|
||||||
print_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; }
|
print_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; }
|
||||||
print_error() { echo -e "${RED}[ERROR]${NC} $1"; }
|
print_error() { echo -e "${RED}[ERROR]${NC} $1"; }
|
||||||
|
|
||||||
VALID_INTEGRATIONS=("litellm" "pydantic-ai" "crewai" "ag2" "ai-sdk" "chat" "openclaw" "langgraph" "llamaindex" "nemoclaw" "strands" "claude-code" "codex" "hermes" "autogen")
|
VALID_INTEGRATIONS=("litellm" "pydantic-ai" "crewai" "ag2" "ai-sdk" "chat" "openclaw" "langgraph" "llamaindex" "nemoclaw" "strands" "claude-code" "codex" "hermes" "autogen" "paperclip")
|
||||||
|
|
||||||
usage() {
|
usage() {
|
||||||
print_error "Usage: $0 <integration> <version>"
|
print_error "Usage: $0 <integration> <version>"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue