Playbook Agent: query catalog (3/8)#574
Draft
rashmiraghunandan wants to merge 3 commits into
Draft
Conversation
Introduce the foundational type system for the playbook agent framework:
playbookTypes.ts — TypeScript types for declarative investigation playbooks:
- SemanticVersion with parsing and formatting
- CatalogReference for versioned query references (no floating versions)
- Playbook config: evidence ontology, labels, decision logic, confidence
computation weights, output contracts
- PlaybookVerdict and ConfidenceBreakdown result types
- QueryResult/QueryResults for evidence store responses
interfaces.ts — Abstract contracts that organizations implement:
- ILLMAdapter: pluggable LLM provider (OpenAI, Anthropic, local, etc.)
- IEvidenceStore: execute pre-approved queries by catalog ID
- IArtifactStore: insert-only storage for immutable audit trail
- IPlaybookRunner: core runner interface
These types enforce the key safety property: playbooks reference queries by
catalog_id@version and never contain SQL, table names, or field mappings.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implement the playbook loader that converts raw YAML/JSON playbook configs
into validated TypeScript Playbook objects.
playbookLoader.ts:
- parsePlaybook(): snake_case YAML input → camelCase TypeScript output
- Cross-reference validation:
- baseline_catalog_refs must be a subset of allowed_catalog_refs
- Deep-dive module query refs must be in the allowed set
- Hard rule outcomes must reference valid label IDs
- Default label must exist in the labels list
- Confidence computation weights must sum to ~1.0
- Validates semantic versions, catalog IDs, input field types, severities
- Rejects forbidden floating versions (latest, head, main, master, current)
playbookLoader.test.ts (7 tests):
- Parses valid playbook with correct type conversion
- Applies default values for optional confidence fields
- Rejects baseline ref not in allowed set
- Rejects invalid default label
- Rejects confidence weights that don't sum to ~1.0
- Rejects invalid semantic versions
- Validates hard rule outcomes against labels
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rashmiraghunandan
requested review from
cassidyjames,
dom-notion,
juanmrad,
julietshen and
vinaysrao1
as code owners
Contributor
📝 WalkthroughWalkthroughThis PR introduces a complete playbook agent framework for declarative risk investigation workflows. It establishes public service contracts, defines comprehensive domain schemas with validation helpers, implements robust YAML/JSON parsing with cross-reference validation, and provides a query catalog system for managing SQL templates by semantic versioned references. ChangesPlaybook Agent Framework
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1 task
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Introduces a playbook agent foundation with typed playbook schemas, YAML parsing/validation, and a query catalog that resolves pre-approved SQL templates with safe parameter binding.
Changes:
- Added
QueryCatalogwith SQL template parsing and positional parameter rendering. - Added playbook schema types + YAML loader with cross-reference validation.
- Added framework interfaces (LLM adapter, evidence store, artifact store, runner) and unit tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| server/services/playbookAgentService/queryCatalog.ts | Adds SQL template parsing, catalog lookup, allowlist checks, and parameter binding renderer |
| server/services/playbookAgentService/queryCatalog.test.ts | Adds unit tests for template parsing, resolving, and SQL rendering |
| server/services/playbookAgentService/playbookTypes.ts | Defines playbook schema/types, semantic versioning, and catalog reference validation |
| server/services/playbookAgentService/playbookLoader.ts | Adds YAML->typed playbook parsing with cross-reference validation |
| server/services/playbookAgentService/playbookLoader.test.ts | Adds unit tests for playbook parsing and validation rules |
| server/services/playbookAgentService/interfaces.ts | Defines adapter interfaces for pluggable infra integrations |
|
|
||
| const METADATA_PATTERN = | ||
| /^--\s*(catalog_id|version|description):\s*(.+)$/; | ||
| const PARAMETER_PATTERN = /:([a-z_][a-z0-9_]*)/g; |
Comment on lines
+144
to
+150
| const renderedSql = entry.sql.replace(PARAMETER_PATTERN, (_match, name: string) => { | ||
| if (!(name in parameters)) { | ||
| throw new Error( | ||
| `Missing required parameter '${name}' for query '${entry.catalogId}'`, | ||
| ); | ||
| } | ||
| bindings.push(parameters[name]); |
Comment on lines
+141
to
+154
| // Build ordered bindings array and replace :param with $N positional params | ||
| const bindings: unknown[] = []; | ||
| let paramIndex = 0; | ||
| const renderedSql = entry.sql.replace(PARAMETER_PATTERN, (_match, name: string) => { | ||
| if (!(name in parameters)) { | ||
| throw new Error( | ||
| `Missing required parameter '${name}' for query '${entry.catalogId}'`, | ||
| ); | ||
| } | ||
| bindings.push(parameters[name]); | ||
| paramIndex++; | ||
| return `$${paramIndex}`; | ||
| }); | ||
|
|
Comment on lines
+141
to
+154
| // Build ordered bindings array and replace :param with $N positional params | ||
| const bindings: unknown[] = []; | ||
| let paramIndex = 0; | ||
| const renderedSql = entry.sql.replace(PARAMETER_PATTERN, (_match, name: string) => { | ||
| if (!(name in parameters)) { | ||
| throw new Error( | ||
| `Missing required parameter '${name}' for query '${entry.catalogId}'`, | ||
| ); | ||
| } | ||
| bindings.push(parameters[name]); | ||
| paramIndex++; | ||
| return `$${paramIndex}`; | ||
| }); | ||
|
|
| } | ||
|
|
||
| get catalogIds(): string[] { | ||
| return [...this.#entries.keys()]; |
Comment on lines
+122
to
+127
| isAllowed(refString: string, playbook: Playbook): boolean { | ||
| const allowedKeys = new Set( | ||
| playbook.allowedCatalogRefs.map(formatCatalogReference), | ||
| ); | ||
| return allowedKeys.has(refString) && this.#entries.has(refString); | ||
| } |
Comment on lines
+252
to
+269
| if ( | ||
| typeof r.use_case_id !== 'string' || | ||
| typeof r.version !== 'string' || | ||
| typeof r.display_name !== 'string' || | ||
| typeof r.description !== 'string' || | ||
| r.evidence_ontology == null || | ||
| !Array.isArray(r.baseline_catalog_refs) || | ||
| !Array.isArray(r.allowed_catalog_refs) || | ||
| r.evidence_request_policy == null || | ||
| !Array.isArray(r.labels) || | ||
| r.decision_logic == null || | ||
| r.confidence_computation == null || | ||
| r.output_contract == null | ||
| ) { | ||
| throw new Error('Playbook is missing required top-level fields'); | ||
| } | ||
|
|
||
| const inputContext: InputContextField[] = (r.input_context ?? []).map( |
Comment on lines
+139
to
+147
| function toCatalogReference(raw: RawCatalogRef): CatalogReference { | ||
| const ref: CatalogReference = { | ||
| catalogId: raw.catalog_id, | ||
| version: parseSemanticVersion(raw.version), | ||
| catalogType: raw.catalog_type as CatalogType, | ||
| }; | ||
| validateCatalogReference(ref); | ||
| return ref; | ||
| } |
Comment on lines
+303
to
+305
| const evidenceRequestPolicy: EvidenceRequestPolicy = { | ||
| querySelectionMode: 'query_id', | ||
| maxRounds: r.evidence_request_policy.max_rounds, |
Comment on lines
+226
to
+237
| const weightSum = | ||
| cc.evidenceCompletenessWeight + | ||
| cc.signalStrengthWeight + | ||
| cc.signalAgreementWeight + | ||
| cc.dataQualityWeight; | ||
| if (Math.abs(weightSum - 1.0) > 0.01) { | ||
| errors.push({ | ||
| field: 'confidenceComputation', | ||
| message: `Confidence weights sum to ${weightSum}, expected ~1.0`, | ||
| }); | ||
| } | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/services/playbookAgentService/playbookLoader.ts`:
- Around line 139-146: The toCatalogReference function currently casts
raw.catalog_type and ignores raw.query_selection_mode; instead validate these
runtime values before building the CatalogReference: check raw.catalog_type is
one of the CatalogType enum values (reject/throw or return an error if not) and
map/parse raw.query_selection_mode into the corresponding SelectionMode value
(or set a safe default) rather than forcing/omitting it; update
toCatalogReference so it uses parseSemanticVersion(raw.version), validates
catalog_type against CatalogType, sets a validated querySelectionMode property
from raw.query_selection_mode, and then calls validateCatalogReference(ref).
- Around line 249-266: The parser currently casts raw to Partial<RawPlaybook>
and then accesses properties which will throw if raw is null or a primitive; add
an explicit non-object guard at the start of parsePlaybook (e.g., verify raw is
non-null and typeof raw === 'object') and throw a clear Error like 'Playbook
must be an object' before performing the existing field checks; reference the
parsePlaybook function and the local variable r to locate where to insert this
guard so subsequent property-type validations (use_case_id, version,
display_name, etc.) are safe.
In `@server/services/playbookAgentService/playbookTypes.ts`:
- Around line 24-37: The parser currently uses Number(...) which accepts
non-canonical forms like "1e2"; update parseSemanticVersion to first validate
each segment string with a strict regex (e.g. /^\d+$/) to allow only ASCII
digits, then convert using parseInt(segment, 10) (instead of Number) and check
for non-negative integers; keep the thrown error messages but reject any segment
that fails the regex so parseSemanticVersion returns a SemanticVersion only for
canonical numeric segments.
In `@server/services/playbookAgentService/queryCatalog.ts`:
- Around line 41-42: The regex PARAMETER_PATTERN is matching PostgreSQL cast
syntax like ::date as a bind parameter; update the pattern to avoid matching a
colon that is directly preceded by another colon by using a negative lookbehind
(e.g. change PARAMETER_PATTERN to use (?<!:):([a-z_][a-z0-9_]*)/g) and replace
any other occurrences of the old pattern (the uses around lines referenced in
the comment) to use this updated pattern so casts like created_at::date are not
treated as parameters.
- Around line 93-97: The constructor for the class building `#entries` currently
silently overwrites duplicates when creating the Map from entries; change it to
detect duplicate `${e.catalogId}@${e.version}` keys while iterating the incoming
entries array (CatalogEntry items) and throw a clear Error if a duplicate key is
encountered (include the duplicate key in the message) instead of allowing the
Map to keep the last entry—this ensures duplicate catalogId@version pairs are
rejected at construction time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ad05515a-6c23-4016-a4d2-e4b0e482d003
📒 Files selected for processing (6)
server/services/playbookAgentService/interfaces.tsserver/services/playbookAgentService/playbookLoader.test.tsserver/services/playbookAgentService/playbookLoader.tsserver/services/playbookAgentService/playbookTypes.tsserver/services/playbookAgentService/queryCatalog.test.tsserver/services/playbookAgentService/queryCatalog.ts
Comment on lines
+139
to
+146
| function toCatalogReference(raw: RawCatalogRef): CatalogReference { | ||
| const ref: CatalogReference = { | ||
| catalogId: raw.catalog_id, | ||
| version: parseSemanticVersion(raw.version), | ||
| catalogType: raw.catalog_type as CatalogType, | ||
| }; | ||
| validateCatalogReference(ref); | ||
| return ref; |
Contributor
There was a problem hiding this comment.
Validate enum-like raw fields instead of casting/overriding.
catalog_type is cast without runtime validation, and query_selection_mode is ignored/forced. Invalid playbook values can pass silently.
Suggested fix
function toCatalogReference(raw: RawCatalogRef): CatalogReference {
+ if (raw.catalog_type !== 'query' && raw.catalog_type !== 'query_bundle') {
+ throw new Error(
+ `Invalid catalog_type '${raw.catalog_type}' for catalog '${raw.catalog_id}'`,
+ );
+ }
const ref: CatalogReference = {
catalogId: raw.catalog_id,
version: parseSemanticVersion(raw.version),
- catalogType: raw.catalog_type as CatalogType,
+ catalogType: raw.catalog_type,
};
validateCatalogReference(ref);
return ref;
}
@@
const evidenceRequestPolicy: EvidenceRequestPolicy = {
- querySelectionMode: 'query_id',
+ querySelectionMode: (() => {
+ if (r.evidence_request_policy.query_selection_mode !== 'query_id') {
+ throw new Error(
+ `Invalid query_selection_mode '${r.evidence_request_policy.query_selection_mode}'`,
+ );
+ }
+ return 'query_id';
+ })(),Also applies to: 303-305
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/services/playbookAgentService/playbookLoader.ts` around lines 139 -
146, The toCatalogReference function currently casts raw.catalog_type and
ignores raw.query_selection_mode; instead validate these runtime values before
building the CatalogReference: check raw.catalog_type is one of the CatalogType
enum values (reject/throw or return an error if not) and map/parse
raw.query_selection_mode into the corresponding SelectionMode value (or set a
safe default) rather than forcing/omitting it; update toCatalogReference so it
uses parseSemanticVersion(raw.version), validates catalog_type against
CatalogType, sets a validated querySelectionMode property from
raw.query_selection_mode, and then calls validateCatalogReference(ref).
Comment on lines
+249
to
+266
| export function parsePlaybook(raw: unknown): Playbook { | ||
| const r = raw as Partial<RawPlaybook>; | ||
|
|
||
| if ( | ||
| typeof r.use_case_id !== 'string' || | ||
| typeof r.version !== 'string' || | ||
| typeof r.display_name !== 'string' || | ||
| typeof r.description !== 'string' || | ||
| r.evidence_ontology == null || | ||
| !Array.isArray(r.baseline_catalog_refs) || | ||
| !Array.isArray(r.allowed_catalog_refs) || | ||
| r.evidence_request_policy == null || | ||
| !Array.isArray(r.labels) || | ||
| r.decision_logic == null || | ||
| r.confidence_computation == null || | ||
| r.output_contract == null | ||
| ) { | ||
| throw new Error('Playbook is missing required top-level fields'); |
Contributor
There was a problem hiding this comment.
Add an explicit non-object guard at parser entry.
parsePlaybook(null) (or primitive input) can fail before structured validation with a generic runtime property-access error.
Suggested fix
export function parsePlaybook(raw: unknown): Playbook {
+ if (typeof raw !== 'object' || raw === null) {
+ throw new Error('Playbook must be an object');
+ }
const r = raw as Partial<RawPlaybook>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function parsePlaybook(raw: unknown): Playbook { | |
| const r = raw as Partial<RawPlaybook>; | |
| if ( | |
| typeof r.use_case_id !== 'string' || | |
| typeof r.version !== 'string' || | |
| typeof r.display_name !== 'string' || | |
| typeof r.description !== 'string' || | |
| r.evidence_ontology == null || | |
| !Array.isArray(r.baseline_catalog_refs) || | |
| !Array.isArray(r.allowed_catalog_refs) || | |
| r.evidence_request_policy == null || | |
| !Array.isArray(r.labels) || | |
| r.decision_logic == null || | |
| r.confidence_computation == null || | |
| r.output_contract == null | |
| ) { | |
| throw new Error('Playbook is missing required top-level fields'); | |
| export function parsePlaybook(raw: unknown): Playbook { | |
| if (typeof raw !== 'object' || raw === null) { | |
| throw new Error('Playbook must be an object'); | |
| } | |
| const r = raw as Partial<RawPlaybook>; | |
| if ( | |
| typeof r.use_case_id !== 'string' || | |
| typeof r.version !== 'string' || | |
| typeof r.display_name !== 'string' || | |
| typeof r.description !== 'string' || | |
| r.evidence_ontology == null || | |
| !Array.isArray(r.baseline_catalog_refs) || | |
| !Array.isArray(r.allowed_catalog_refs) || | |
| r.evidence_request_policy == null || | |
| !Array.isArray(r.labels) || | |
| r.decision_logic == null || | |
| r.confidence_computation == null || | |
| r.output_contract == null | |
| ) { | |
| throw new Error('Playbook is missing required top-level fields'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/services/playbookAgentService/playbookLoader.ts` around lines 249 -
266, The parser currently casts raw to Partial<RawPlaybook> and then accesses
properties which will throw if raw is null or a primitive; add an explicit
non-object guard at the start of parsePlaybook (e.g., verify raw is non-null and
typeof raw === 'object') and throw a clear Error like 'Playbook must be an
object' before performing the existing field checks; reference the parsePlaybook
function and the local variable r to locate where to insert this guard so
subsequent property-type validations (use_case_id, version, display_name, etc.)
are safe.
Comment on lines
+24
to
+37
| export function parseSemanticVersion(input: string): SemanticVersion { | ||
| const parts = input.split('.'); | ||
| if (parts.length !== 3) { | ||
| throw new Error( | ||
| `Invalid semantic version format: '${input}'. Expected format: 'X.Y.Z'`, | ||
| ); | ||
| } | ||
| const [major, minor, patch] = parts.map(Number); | ||
| if ([major, minor, patch].some((n) => !Number.isInteger(n) || n < 0)) { | ||
| throw new Error( | ||
| `Invalid semantic version format: '${input}'. All parts must be non-negative integers.`, | ||
| ); | ||
| } | ||
| return { major, minor, patch }; |
Contributor
There was a problem hiding this comment.
Harden semantic-version parsing to reject non-canonical numeric segments.
The current Number(...) conversion accepts inputs like 1e2.0.0, which should not be valid semantic versions in this parser.
Suggested fix
export function parseSemanticVersion(input: string): SemanticVersion {
- const parts = input.split('.');
- if (parts.length !== 3) {
- throw new Error(
- `Invalid semantic version format: '${input}'. Expected format: 'X.Y.Z'`,
- );
- }
- const [major, minor, patch] = parts.map(Number);
- if ([major, minor, patch].some((n) => !Number.isInteger(n) || n < 0)) {
+ const match = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)$/.exec(input);
+ if (!match) {
throw new Error(
- `Invalid semantic version format: '${input}'. All parts must be non-negative integers.`,
+ `Invalid semantic version format: '${input}'. Expected format: 'X.Y.Z'`,
);
}
+ const major = Number(match[1]);
+ const minor = Number(match[2]);
+ const patch = Number(match[3]);
return { major, minor, patch };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/services/playbookAgentService/playbookTypes.ts` around lines 24 - 37,
The parser currently uses Number(...) which accepts non-canonical forms like
"1e2"; update parseSemanticVersion to first validate each segment string with a
strict regex (e.g. /^\d+$/) to allow only ASCII digits, then convert using
parseInt(segment, 10) (instead of Number) and check for non-negative integers;
keep the thrown error messages but reject any segment that fails the regex so
parseSemanticVersion returns a SemanticVersion only for canonical numeric
segments.
Implement the query catalog — the trust boundary between the AI agent and
the database. The agent requests evidence by catalog_id@version; it never
writes SQL or sees table names.
queryCatalog.ts:
- parseCatalogEntry(): extract metadata (catalog_id, version, description)
from SQL comment headers and discover :param tokens
- QueryCatalog class:
- resolve(): look up CatalogReference → CatalogEntry
- resolveByString(): look up "catalog_id@version" string
- isAllowed(): validate a ref is in the playbook's allowed set
- renderSql(): substitute :param tokens with positional $N bindings
for safe parameterized execution (prevents SQL injection)
queryCatalog.test.ts (7 tests):
- Parses SQL template with metadata comments
- Throws on missing catalog_id or version
- Deduplicates repeated parameters
- Resolves by CatalogReference and by string
- Renders SQL with positional parameters and bindings array
- Throws on missing required parameter
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rashmiraghunandan
force-pushed
the
playbook-agent/03-query-catalog
branch
from
5b09347 to
3fb3153
Compare
cassidyjames
changed the title
This was referenced May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The query catalog — the trust boundary between the AI agent and the database. The agent requests evidence by
catalog_id@version; it never writes SQL or sees table names.Part 3 of 8 — depends on #573 (playbook loader).
queryCatalog.tsparseCatalogEntry(): extract metadata from SQL comment headers, discover:paramtokensQueryCatalogclass:resolve()/resolveByString(): look up versioned catalog entriesisAllowed(): validate a ref is in the playbook's allowed setrenderSql(): substitute:paramtokens with positional$Nbindings (prevents SQL injection)Template format
queryCatalog.test.ts— 7 tests$NparametersTest plan
npm test -- --testPathPattern=queryCatalogpasses (7 tests)Summary by CodeRabbit
New Features
Tests