feat: prompt-pipeline library and formatters#22
Conversation
SafeDep Report SummaryPackage Details
This report is generated by SafeDep Github App |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new typed, slot-based prompt assembly pipeline (@kagura/prompt-pipeline) plus provider-specific formatters for Claude and OpenAI, and adds a dedicated test suite to validate core behavior (ordering, injection, assembling, formatting, tracing).
Changes:
- Added
@kagura/prompt-pipelinewith slot writers, plugin definition helpers, pipeline runner, assembler, and error types. - Added
@kagura/prompt-formatter-claudeand@kagura/prompt-formatter-openaipackages implementing API-specific payload formatting. - Updated Vitest/TS test config to resolve the new workspace packages and added extensive unit/integration tests.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds Vitest resolve aliases for the new workspace packages. |
| tsconfig.tests.json | Adds TS path mappings for the new workspace packages for tests/typechecking. |
| tests/prompt-pipeline/types.test.ts | Tests exported core types and error classes. |
| tests/prompt-pipeline/slot-writer.test.ts | Tests SlotWriter segment/image behavior. |
| tests/prompt-pipeline/plugin.test.ts | Tests definePlugin behavior and inject schema preservation. |
| tests/prompt-pipeline/pipeline.test.ts | Tests ordering, injection validation, tracing, and runWith formatting. |
| tests/prompt-pipeline/integration.test.ts | End-to-end tests combining pipeline + Claude/OpenAI formatters. |
| tests/prompt-pipeline/assembler.test.ts | Tests slot-to-message assembly rules and image attachment. |
| tests/prompt-formatter-openai.test.ts | Tests OpenAI formatter system-message behavior. |
| tests/prompt-formatter-claude.test.ts | Tests Claude formatter multipart content for images. |
| packages/prompt-pipeline/tsdown.config.ts | Build config for the new pipeline package. |
| packages/prompt-pipeline/tsconfig.json | TS config for the new pipeline package. |
| packages/prompt-pipeline/package.json | Declares the new pipeline package and its build/typecheck scripts. |
| packages/prompt-pipeline/src/types.ts | Defines slots, message types, plugin definitions, formatter interface. |
| packages/prompt-pipeline/src/slot-writer.ts | Implements SlotWriter with segment + image collection. |
| packages/prompt-pipeline/src/plugin.ts | Adds definePlugin helper. |
| packages/prompt-pipeline/src/pipeline.ts | Implements pipeline execution, injection parsing, tracing, and assembly. |
| packages/prompt-pipeline/src/index.ts | Public exports for the pipeline package. |
| packages/prompt-pipeline/src/errors.ts | Adds pipeline-related error classes. |
| packages/prompt-pipeline/src/assembler.ts | Assembles writers + history into a PromptResult. |
| packages/prompt-formatter-openai/tsdown.config.ts | Build config for the OpenAI formatter package. |
| packages/prompt-formatter-openai/tsconfig.json | TS config for the OpenAI formatter package. |
| packages/prompt-formatter-openai/package.json | Declares the OpenAI formatter package and scripts. |
| packages/prompt-formatter-openai/src/index.ts | Implements OpenAI payload formatting. |
| packages/prompt-formatter-claude/tsdown.config.ts | Build config for the Claude formatter package. |
| packages/prompt-formatter-claude/tsconfig.json | TS config for the Claude formatter package. |
| packages/prompt-formatter-claude/package.json | Declares the Claude formatter package and scripts. |
| packages/prompt-formatter-claude/src/index.ts | Implements Claude payload formatting (including images). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| alias: { | ||
| '@kagura/prompt-pipeline': new URL('./packages/prompt-pipeline/src/index.ts', import.meta.url) | ||
| .pathname, | ||
| '@kagura/prompt-formatter-claude': new URL( | ||
| './packages/prompt-formatter-claude/src/index.ts', | ||
| import.meta.url, | ||
| ).pathname, | ||
| '@kagura/prompt-formatter-openai': new URL( | ||
| './packages/prompt-formatter-openai/src/index.ts', | ||
| import.meta.url, | ||
| ).pathname, |
There was a problem hiding this comment.
new URL(...).pathname is not reliably a filesystem path on all platforms (notably Windows drive-letter paths and URL-encoding). Prefer fileURLToPath(new URL(..., import.meta.url)) (and keep the result as a normal path string) for the Vitest alias targets.
| function sortPluginsBySlot(plugins: PluginDef[]): PluginDef[] { | ||
| const slotIndex = new Map(SLOT_ORDER.map((s, i) => [s, i])); | ||
| const indexed = plugins.map((p, registrationOrder) => ({ p, registrationOrder })); | ||
| indexed.sort((a, b) => { | ||
| const slotDiff = (slotIndex.get(a.p.slot) ?? 0) - (slotIndex.get(b.p.slot) ?? 0); | ||
| if (slotDiff !== 0) return slotDiff; | ||
| return a.registrationOrder - b.registrationOrder; | ||
| }); | ||
| return indexed.map((x) => x.p); |
There was a problem hiding this comment.
sortPluginsBySlot currently treats an unknown plugin.slot as index 0 via ?? 0, which silently reorders invalid plugins into the system slot. Since PipelineConfigError exists, consider validating plugin.slot against SLOT_ORDER and throwing a config error (or at least sorting unknown slots to the end) to avoid surprising execution order at runtime.
| export interface PluginDef<TInject extends z.ZodType = z.ZodType> { | ||
| inject?: TInject; | ||
| name: string; | ||
| process: (ctx: SlotWriter, deps: z.infer<TInject>) => Promise<void>; |
There was a problem hiding this comment.
PluginDef.process types deps as z.infer<TInject> even when inject is omitted, but createPipeline will pass undefined for non-inject plugins. Consider modeling this in the types (e.g., overloads/conditional types so plugins without inject receive deps: undefined) to prevent authors from accidentally destructuring deps at runtime without an inject schema.
| export interface PluginDef<TInject extends z.ZodType = z.ZodType> { | |
| inject?: TInject; | |
| name: string; | |
| process: (ctx: SlotWriter, deps: z.infer<TInject>) => Promise<void>; | |
| type PluginDeps<TInject extends z.ZodType | undefined> = | |
| [TInject] extends [z.ZodType] ? z.infer<TInject> : undefined; | |
| export interface PluginDef<TInject extends z.ZodType | undefined = undefined> { | |
| inject?: TInject; | |
| name: string; | |
| process: (ctx: SlotWriter, deps: PluginDeps<TInject>) => Promise<void>; |
| export function definePlugin<TInjectSchema extends z.ZodType>(def: { | ||
| name: string; | ||
| slot: Slot; | ||
| inject?: TInjectSchema; | ||
| process: (ctx: SlotWriter, deps: z.infer<TInjectSchema>) => Promise<void>; | ||
| }): PluginDef<TInjectSchema> { |
There was a problem hiding this comment.
definePlugin requires deps: z.infer<TInjectSchema> even when inject is not provided, but the pipeline passes undefined in that case. Consider adding overloads (inject present vs absent) so TypeScript enforces the correct deps type and avoids an implicit any cast in createPipeline.
| export function definePlugin<TInjectSchema extends z.ZodType>(def: { | |
| name: string; | |
| slot: Slot; | |
| inject?: TInjectSchema; | |
| process: (ctx: SlotWriter, deps: z.infer<TInjectSchema>) => Promise<void>; | |
| }): PluginDef<TInjectSchema> { | |
| export function definePlugin(def: { | |
| name: string; | |
| slot: Slot; | |
| process: (ctx: SlotWriter, deps: undefined) => Promise<void>; | |
| }): PluginDef<z.ZodUndefined>; | |
| export function definePlugin<TInjectSchema extends z.ZodType>(def: { | |
| name: string; | |
| slot: Slot; | |
| inject: TInjectSchema; | |
| process: (ctx: SlotWriter, deps: z.infer<TInjectSchema>) => Promise<void>; | |
| }): PluginDef<TInjectSchema>; | |
| export function definePlugin(def: { | |
| name: string; | |
| slot: Slot; | |
| inject?: z.ZodType; | |
| process: (ctx: SlotWriter, deps: unknown) => Promise<void>; | |
| }): PluginDef<z.ZodType> { |
Innei
left a comment
There was a problem hiding this comment.
PR Review: prompt-pipeline library and formatters
已详阅全部 diff(+1266 行,11 commits)。以下为 review 结果。
架构评价
Slot + Plugin + Formatter 三层抽象清晰,Zod inject 验证 + trace 记录完备。整体设计可取。测试覆盖充分(323 tests)。
🔴 Critical
1. pipeline.ts:474 — deps as any 绕过类型安全
await plugin.process(writer, deps as any);deps 声明为 unknown,强转 any 传入 process。若 plugin 声明了 inject schema 但 parse 失败后被 catch 重新抛出,此处仍以 any 传入 — 类型信息全失。应利用 PluginDef 的泛型参数在调用侧保留类型,或至少用 type assertion 收窄为 z.infer<TInject> | undefined。
🟡 Important
1. openaiFormatter 静默丢弃 images
ResolvedMessage.images 在 OpenAI formatter 中被完全忽略。若上游 plugin 写入 image asset,此处无警告亦无 error。建议至少 log 一条 warning,或在类型层面以 Formatter<Omit<ResolvedMessage, 'images'>> 明确表达此限制。
2. assembler.ts:323-324 — afterSystem 双重计算
const afterSystemMessages = segmentsToMessages(writers.get('afterSystem'));
const afterSystem = writers.get('afterSystem')?.getSegments() ?? [];segmentsToMessages 内部已调用 getSegments(),此处重复求值。且 PromptResult.afterSystem(string[])与 messages 中 afterSystem 段落数据冗余 — consumer 不知应读哪个字段。
3. prompt-pipeline/tsconfig.json 引入 "DOM" lib
仅为 performance.now() 引入 DOM 类型。此包可能在 Node / edge runtime 运行。建议改用 node:perf_hooks 的 performance,或用 Date.now() 替代以消除 DOM 依赖。
4. 插件 process 异常无 partial result
若第 N 个 plugin 抛错,前 N-1 个 plugin 的写入结果全部丢失。对于长管道而言,部分结果(含 trace)有助于调试。建议 catch 后返回 Partial<PromptResult> 或至少将 trace 附加到 error 对象上。
5. pipeline.ts:448 — input parse 错误未包装
const parsed = config.input.parse(input);Zod parse 失败抛 ZodError,但未被 PipelineConfigError 包装。consumer 需同时 catch 两种 error 类型。建议统一包装。
🟢 Minor
1. 单测直接 import 内部模块
assembler.test.ts / slot-writer.test.ts 直接 import ../../packages/prompt-pipeline/src/slot-writer.js,绕过公共 API。此耦合使内部重构时测试亦需同步修改。建议仅通过 @kagura/prompt-pipeline 公共导出测试。
2. integration.test.ts:1018 — as any cast
expect((payload.messages[1] as any).content).toContain('PREFERENCES');多处 as any 表明 ClaudeMessage 的 content 联合类型(string | ClaudeContentBlock[])未在 formatter 返回类型上精确表达。可改善类型推导消除 as any。
3. peerDependencies zod 版本范围过宽
"zod": ">=4.0.0" 允许任何 Zod 4.x/5.x。若 Zod 5 API 不兼容,consumer 升级后此包将静默 break。建议收紧为 "4.x" 或使用 "^4.0.0"。
总结
架构设计良,测试覆盖充分。主要关注点:as any 类型漏洞、DOM lib 依赖、OpenAI formatter 丢 image、冗余字段。建议修完 Critical + Important 后再合。
…dation, and tracing
…ng, never> type conflict Pipeline.run() and Pipeline.runWith() now accept messages via an optional second/third parameter instead of merging into input. Also adds @kagura/* path aliases to tsconfig.tests.json.
705a230 to
26c3274
Compare




Summary
packages/prompt-pipeline: typed pipeline with slots, plugins,SlotWriter, assembler, tracing, and validation hooks.prompt-formatter-claudeandprompt-formatter-openaipackages for API-specific message formatting.src/agent/providers/claude-code/prompt-pipeline/with end-to-end tests.Test plan
pnpm buildpnpm test(323 tests)Made with Cursor