(WIP) feat: add support drivers for afs, providing multiple views for the node.#855
Open
lban2049 wants to merge 23 commits into
Open
(WIP) feat: add support drivers for afs, providing multiple views for the node.#855lban2049 wants to merge 23 commits into
lban2049 wants to merge 23 commits into
Conversation
…adata retrieval in SQLite store
…ation agent and storage mapping
…n source and view operations
|
WalkthroughThis changeset introduces a comprehensive view-based architecture to the AFS (Abstract File System) with driver support for content transformation. The implementation adds metadata management with SQLite storage, view processing with caching and state management, and an i18n driver for multilingual file translation. The system replaces context-based presets with a driver architecture, enabling dynamic content projections while maintaining backward compatibility through fallback mechanisms. Changes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (29)
- afs/core/src/afs.ts (7)
- afs/core/src/index.ts (1)
- afs/core/src/metadata/index.ts (1)
- afs/core/src/metadata/migrate.ts (1)
- afs/core/src/metadata/migrations/001-init.ts (1)
- afs/core/src/metadata/models/index.ts (1)
- afs/core/src/metadata/models/source-metadata.ts (1)
- afs/core/src/metadata/models/view-metadata.ts (1)
- afs/core/src/metadata/store.ts (1)
- afs/core/src/metadata/type.ts (1)
- afs/core/src/type.ts (2)
- afs/core/src/view-processor.ts (1)
- afs/core/src/view-schema.ts (1)
- afs/core/test/view-driver.test.ts (1)
- afs/i18n-driver/scripts/tsconfig.build.cjs.json (1)
- afs/i18n-driver/scripts/tsconfig.build.dts.json (1)
- afs/i18n-driver/scripts/tsconfig.build.esm.json (1)
- afs/i18n-driver/scripts/tsconfig.build.json (1)
- afs/i18n-driver/src/default-translation-agent.ts (1)
- afs/i18n-driver/src/driver.ts (1)
- afs/i18n-driver/src/index.ts (1)
- afs/i18n-driver/src/storage.ts (1)
- afs/i18n-driver/test/i18n-driver.test.ts (1)
- afs/i18n-driver/tsconfig.json (1)
- packages/cli/src/utils/load-aigne.ts (1)
- packages/core/src/loader/agent-yaml.ts (9)
- packages/core/src/loader/index.ts (7)
- packages/core/src/prompt/skills/afs/read.ts (1)
- packages/core/test/loader/agent-yaml.test.ts (1)
Files ignored due to filter (9)
- afs/core/package.json
- afs/i18n-driver.md
- afs/i18n-driver/CHANGELOG.md
- afs/i18n-driver/README.md
- afs/i18n-driver/package.json
- packages/cli/package.json
- packages/core/test-agents/test-afs-driver.yaml
- packages/core/test-agents/test-afs-storage.yaml
- pnpm-lock.yaml
Files skipped from review due to trivial changes (6)
- afs/i18n-driver/scripts/tsconfig.build.cjs.json
- afs/i18n-driver/scripts/tsconfig.build.dts.json
- afs/i18n-driver/scripts/tsconfig.build.esm.json
- afs/i18n-driver/scripts/tsconfig.build.json
- afs/i18n-driver/tsconfig.json
- packages/core/test/loader/agent-yaml.test.ts
Review comments generated (66)
- Review: 12
- LGTM: 54
提示
与 
AIGNE CodeSmith (@codesmith) 聊天
- 回复此机器人留下的审查评论以提出后续问题。审查评论是对差异或文件的评论。
- 通过在回复中标记
@codesmith邀请机器人加入审查评论链。
代码建议
- 机器人可能会提出代码建议，但在提交前请仔细审查它们，因为行号范围可能会不对齐。
- 你可以编辑机器人做出的评论，并在建议稍有偏差时手动调整。
暂停增量审查
- 在 PR 描述中的任何位置添加
@codesmith: ignore以暂停机器人的进一步审查。
| pathIdx: index("idx_view_path").on(table.path), | ||
| stateIdx: index("idx_view_state").on(table.state), | ||
| derivedFromIdx: index("idx_view_derived_from").on(table.derivedFrom), | ||
| uniqueModulePathView: index("unique_module_path_view").on(table.module, table.path, table.view), |
There was a problem hiding this comment.
The index name suggests it should be unique, but it's created as a regular index instead of a unique index.
- uniqueModulePathView: index("unique_module_path_view").on(table.module, table.path, table.view),
+ uniqueModulePathView: index("unique_module_path_view").on(table.module, table.path, table.view).unique(),
Comment on lines
+44
to
+48
| /** | ||
| * View represents different projections of the same file | ||
| * V1: Only language dimension is implemented | ||
| * Future: format, policy, variant dimensions | ||
| */ |
There was a problem hiding this comment.
The comment contains a grammatical error and unclear wording.
- * View represents different projections of the same file
- * V1: Only language dimension is implemented
- * Future: format, policy, variant dimensions
+ * View represents different projections of the same file.
+ * V1: Only the language dimension is implemented.
+ * Future: format, policy, variant dimensions will be added.
Comment on lines
+56
to
+61
| /** | ||
| * Wait strategy for view processing | ||
| * - strict: Wait for view generation to complete before returning | ||
| * - fallback: Return source immediately and trigger background generation | ||
| */ | ||
| export type WaitStrategy = "strict" | "fallback"; |
There was a problem hiding this comment.
The comment contains a grammatical error in the fallback description.
- * - fallback: Return source immediately and trigger background generation
+ * - fallback: Returns source immediately and triggers background generation
Comment on lines
+63
to
+69
| /** | ||
| * View status in read result | ||
| * Indicates whether the requested view was returned or fell back to source | ||
| */ | ||
| export interface ViewStatus { | ||
| fallback?: boolean; // true = returned source content, view is being generated in background | ||
| } |
There was a problem hiding this comment.
The comment contains unclear wording that could be confusing.
- * Indicates whether the requested view was returned or fell back to source
+ * Indicates whether the requested view was returned or if it fell back to the source| * Process a view (generate or regenerate) | ||
| * V1: Direct execution without job deduplication | ||
| */ | ||
| async processView(module: AFSModule, path: string, view: View, context: any): Promise<AFSEntry> { |
There was a problem hiding this comment.
The context parameter should have a more specific type instead of any for better type safety.
- async processView(module: AFSModule, path: string, view: View, context: any): Promise<AFSEntry> {
+ async processView(module: AFSModule, path: string, view: View, context: unknown): Promise<AFSEntry> {| }, | ||
| ); | ||
|
|
||
| expect(result.result.content).toBe("Hello，World！This is aTest。"); |
There was a problem hiding this comment.
There's a spacing issue in the expected translation result.
- expect(result.result.content).toBe("Hello，World！This is aTest。");
+ expect(result.result.content).toBe("Hello，World！This is a Test。");| // Function config - we can't know drivers without calling it | ||
| // Return empty array as we can't determine drivers at construction time | ||
| return []; | ||
| } |
There was a problem hiding this comment.
There's a missing closing brace for the if statement.
- return [];
- }
+ return [];
+ }| "Wait strategy: 'strict' waits for view generation (default), 'fallback' returns source immediately", | ||
| ), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation - should use 2 spaces to match the rest of the file.
- }
+ }| .split("\n") | ||
| .map((line, idx) => `${idx + 1}| ${line}`) | ||
| .join("\n"); | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation - should use 2 spaces to match the rest of the file.
- }
+ }
Comment on lines
149
to
160
| return { | ||
| status: "success", | ||
| tool: "afs_read", | ||
| path: input.path, | ||
| totalLines, | ||
| returnedLines, | ||
| truncated, | ||
| offset, | ||
| message, | ||
| withLineNumbers: input.withLineNumbers, | ||
| ...result, | ||
| data: { | ||
| data: result.data && { | ||
| ...result.data, | ||
| content: processedContent, | ||
| content, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation - should use 2 spaces to match the rest of the file.
- return {
- status: "success",
- tool: "afs_read",
- path: input.path,
- withLineNumbers: input.withLineNumbers,
- ...result,
- data: result.data && {
- ...result.data,
- content,
- },
- };
+ return {
+ status: "success",
+ tool: "afs_read",
+ path: input.path,
+ withLineNumbers: input.withLineNumbers,
+ ...result,
+ data: result.data && {
+ ...result.data,
+ content,
+ },
+ };…metadata handling
…AI-generated images and metadata handling
There was a problem hiding this comment.
Pull request overview
This PR adds support for AFS drivers, enabling a view-based content transformation system. The primary use case is providing multiple language versions of files through an i18n driver, which can automatically translate content on demand.
Key Changes:
- Introduced a driver architecture allowing files to be processed through configurable transformations (views)
- Implemented i18n driver for automatic file translation across multiple languages
- Added metadata storage system to track source files and generated views
- Extended AFS read operations to support view parameters and wait strategies
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/loader/index.ts | Added driver loading and initialization logic to support AFS drivers configuration |
| packages/core/src/loader/agent-yaml.ts | Extended YAML schema to support drivers and storage configuration |
| packages/core/src/prompt/skills/afs/read.ts | Added view and wait parameters to read skill with viewStatus output |
| packages/cli/src/utils/load-aigne.ts | Registered i18n driver as available driver for CLI |
| afs/i18n-driver/* | New package implementing i18n translation driver |
| afs/core/src/* | Core AFS extensions for view processing, metadata, and driver support |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+238
to
+267
| const modules: AFSModule[] = []; | ||
| for (const m of agent.afs.modules || []) { | ||
| const moduleName = typeof m === "string" ? m : m.module; | ||
|
|
||
| const mod = options?.afs?.availableModules?.find( | ||
| (mod) => mod.module === moduleName || mod.alias?.includes(moduleName), | ||
| ); | ||
| if (!mod) throw new Error(`AFS module not found: ${typeof m === "string" ? m : m.module}`); | ||
|
|
||
| const module = await mod.load({ | ||
| filepath: path, | ||
| parsed: typeof m === "string" ? {} : m.options, | ||
| }); | ||
|
|
||
| modules.push(module); | ||
| } | ||
|
|
||
| // Create drivers | ||
| const drivers: AFSDriver[] = []; | ||
| for (const d of agent.afs.drivers || []) { | ||
| const driverName = typeof d === "string" ? d : d.driver; | ||
|
|
||
| const drv = options?.afs?.availableDrivers?.find( | ||
| (drv) => drv.driver === driverName || drv.alias?.includes(driverName), | ||
| ); | ||
| if (!drv) throw new Error(`AFS driver not found: ${typeof d === "string" ? d : d.driver}`); | ||
|
|
||
| const driver = await drv.load({ | ||
| parsed: typeof d === "string" ? {} : d.options, | ||
| }); |
There was a problem hiding this comment.
Line 267 pushes module to the modules array instead of driver to the drivers array. This should be drivers.push(driver); to correctly populate the drivers array.
Comment on lines
+354
to
+357
| async cleanupFailedViews(_olderThan?: Date): Promise<void> { | ||
| const db = await this.db; | ||
| // Note: drizzle doesn't have a direct < operator for dates | ||
| // For now, we delete all failed views regardless of age |
There was a problem hiding this comment.
The olderThan parameter is prefixed with underscore indicating it's unused, but the method signature doesn't match the interface requirement. Either implement the date filtering logic or document why it's not implemented and keep the parameter for interface compliance.
Suggested change
| async cleanupFailedViews(_olderThan?: Date): Promise<void> { | |
| const db = await this.db; | |
| // Note: drizzle doesn't have a direct < operator for dates | |
| // For now, we delete all failed views regardless of age | |
| /** | |
| * Remove metadata entries for failed views. | |
| * | |
| * @param olderThan - Optional timestamp from the MetadataStore interface. Currently | |
| * ignored in this SQLite implementation: due to limitations in the underlying | |
| * query builder, we do not yet filter by age and instead delete all failed views. | |
| * The parameter is kept for interface compatibility and future extension. | |
| */ | |
| async cleanupFailedViews(olderThan?: Date): Promise<void> { | |
| const db = await this.db; | |
| // Explicitly ignore `olderThan` for now; see documentation above. | |
| void olderThan; | |
| // Note: drizzle doesn't have a direct < operator for dates. | |
| // For now, we delete all failed views regardless of age. |
Comment on lines
+31
to
+35
| // translationAgent: optionalize( | ||
| // z.custom<Agent<TranslationInput, TranslationOutput>>( | ||
| // (value) => value instanceof Agent<TranslationInput, TranslationOutput>, | ||
| // ), | ||
| // ), |
There was a problem hiding this comment.
Commented-out code should be removed rather than left in the codebase. If the translationAgent field validation is intentionally excluded, add a comment explaining why.
Suggested change
| // translationAgent: optionalize( | |
| // z.custom<Agent<TranslationInput, TranslationOutput>>( | |
| // (value) => value instanceof Agent<TranslationInput, TranslationOutput>, | |
| // ), | |
| // ), | |
| // Note: translationAgent is intentionally not validated here; the Agent type | |
| // is provided and checked at the type level by consumers of this driver. |
Comment on lines
+24
to
+28
| const i18nDriver = new I18nDriver({ | ||
| context, | ||
| defaultSourceLanguage: "zh", | ||
| supportedLanguages: ["en", "ja", "ko"], | ||
| }); |
There was a problem hiding this comment.
The README shows context as a required parameter in the I18nDriver constructor options, but the actual implementation (driver.ts:63) doesn't include context in the I18nDriverOptions interface. The documentation is inconsistent with the implementation.
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.
Related Issue
Major Changes
Screenshots
Test Plan
Checklist
Summary by AIGNE
Release Notes
New Feature:
Refactor: