[#777] implement-workflow-builder#780
Open
nrslib wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughPR ChangesWorkflow Builder Feature
🎯 4 (Complex) | ⏱️ ~60 minutes🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/interactive/conversationLoop.ts (1)
242-257:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
/accept・/playの拒否メッセージを i18n 化してください。ここだけ固定英語文字列なので、
jaロケールでも英語表示になります。既存どおりinteractive.uiのラベル経由に揃えた方がよいです。🤖 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 `@src/features/interactive/conversationLoop.ts` around lines 242 - 257, Replace the hardcoded English refusal message inside the blocks that check strategy.disableDirectExecuteCommands (both where SlashCommand.Accept and SlashCommand.Play are handled) with an i18n lookup via the existing interactive.ui labels instead of the literal 'Use /go to apply confirmed changes.'; call the same ui label used elsewhere (e.g., ui.useGoToApply or add that key under interactive.ui/locales if missing) and pass the result to info() so the message is localized across locales.
🤖 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 `@src/features/interactive/conversationLoop.ts`:
- Around line 364-366: The code displays the wrong message when resume is
disabled: in the branch checking strategy.enableResumeCommand === false it
currently calls ui.retryUnavailable; change that call to ui.resumeUnavailable
(or add that i18n string if missing) so the user sees a resume-specific message;
update any tests or callers referencing ui.retryUnavailable in this branch to
use ui.resumeUnavailable and keep the continue behavior in the conversationLoop
control flow.
In `@src/features/workflowAuthoring/builder/approval.ts`:
- Around line 54-83: The current fallback to latestAssistantCandidatePaths when
the user says `yes`/`ok` causes implicit approvals; change the logic so that
when hasMentionedCandidate is false you only use latestAssistantCandidatePaths
if the prior assistant message was an explicit approval request (use
isApprovalRequestMessage(...) on the assistant message that produced
latestAssistantCandidatePaths); otherwise require explicit path mentions and
skip approval. Update the branch around approved = hasMentionedCandidate ?
mentioned : latestAssistantCandidatePaths to check isApprovalRequestMessage(...)
and call removeApprovedBuilderCandidatePaths / addApprovedBuilderCandidatePath
accordingly.
In `@src/features/workflowAuthoring/builder/command.ts`:
- Around line 91-99: selectRequiredOption currently throws when selectOption
returns no selection, which causes the whole builderWorkflowCommand to reject on
simple user cancel; change selectRequiredOption to stop throwing and instead
return a nullable result (e.g., Promise<T | undefined>) when no selection is
made, so callers can handle an early return/abort gracefully; update the
selectRequiredOption signature and body to return undefined on cancel and then
update callers such as builderWorkflowCommand to check for falsy/undefined and
return early instead of relying on exceptions.
- Around line 132-141: When parseBuilderManifestForGo(...) returns falsy or
findBuilderChangeViolation(...) returns a plannedViolation, append the
validation feedback into goContext.history (same approach as in workflow
doctor/apply) instead of only logging to the terminal: call
buildBuilderValidationFeedback(...) with the appropriate inputs (e.g.,
options.scope, manifest or manifestChanges, and the error/violation message) and
push the result onto goContext.history before returning null; apply the same
change for the other block referenced around the 213-225 range so both
manifest-parse failures and approval-scope violations record feedback in
history.
In `@src/features/workflowAuthoring/builder/files.ts`:
- Around line 18-26: listFilesRecursive currently calls readdirSync on rootDir
even when rootDir is a regular file; add a directory check like the existing
symlink guard to return [] when lstatSync(rootDir).isDirectory() is false. In
other words, in function listFilesRecursive, after existsSync(rootDir) and
before calling readdirSync(rootDir), call lstatSync(rootDir) and if
!isDirectory() return []; this mirrors the symlink exclusion and prevents
ENOTDIR errors from readdirSync.
In `@src/features/workflowAuthoring/builder/manifest.ts`:
- Line 21: 現在の BUILDER_MANIFEST_JSON_BLOCK が最初のコードフェンス（json 指定なし）もマッチしてしまい説明用の
```yaml などで JSON.parse が失敗するので、まず ```json
を優先して抽出し、未指定フェンスはレスポンス全体が単一フェンスだった場合のみ許可するように修正してください: 定数を
BUILDER_MANIFEST_JSON_BLOCK = /```json\s*([\s\S]*?)```/i とし、別で
BUILDER_MANIFEST_SINGLE_FENCE = /^\s*```(?:json)?\s*([\s\S]*?)```\s*$/i
を定義、extractBuilderManifestJson(content: string) 内でまず BUILDER_MANIFEST_JSON_BLOCK
を試し、マッチしなければ BUILDER_MANIFEST_SINGLE_FENCE を試してマッチ時はそのグループを返し（trim
する）、どちらにもマッチしなければ元の content.trim() を返すようにしてください（関数名 extractBuilderManifestJson
と定数名を参照）。
In `@src/features/workflowAuthoring/builder/promptContext.ts`:
- Around line 71-88: The current buildScopeRelatedGraph function (and related
logic used at lines ~165-193) inlines full workflow and facet bodies for every
candidate, causing quadratic token growth; change buildScopeRelatedGraph to only
emit per-candidate metadata (path, relation, reason) by replacing/adjusting the
use of formatRelatedCandidates (or creating a new formatter) so it returns only
"path / relation / reason" entries, leaving out workflow/facet text, and ensure
buildRelatedWorkflowAnalysis consumers still provide candidate identifiers so
the actual bodies can be retrieved later on explicit "Read" requests during the
conversation.
- Around line 42-48: The buildBuilderSystemPrompt is injecting the Japanese
style guide because loadStyleGuide() (and similarly loadYamlSchema()) currently
always reads getLanguageResourcesDir('ja'); update buildBuilderSystemPrompt to
pass the lang through to the resource loaders so they load the matching language
(e.g., change calls to loadStyleGuide(lang) and loadYamlSchema(lang) or modify
those helpers to accept a language parameter), and apply the same change to the
other builder functions in the file (the block around lines 108-126) so English
prompts receive English resources and Japanese prompts receive Japanese
resources.
In `@src/features/workflowAuthoring/builder/validation.ts`:
- Around line 31-38: The loop over listBuilderTargetWorkflows currently calls
WorkflowConfigRawSchema.parse(parseYamlContent(workflow.path)) directly which
throws on any unreadable/invalid YAML and aborts facet-change resolution; wrap
the parse step in a try/catch (or reuse the same “skip unreadable workflow”
logic used by parseWorkflowForApprovalScope) so that if parseYamlContent or
WorkflowConfigRawSchema.parse fails you log or warn and continue to the next
workflow, then only call resolveUsedFacetPaths and potentially add
resolve(workflow.path) to targets for successfully parsed workflows.
---
Outside diff comments:
In `@src/features/interactive/conversationLoop.ts`:
- Around line 242-257: Replace the hardcoded English refusal message inside the
blocks that check strategy.disableDirectExecuteCommands (both where
SlashCommand.Accept and SlashCommand.Play are handled) with an i18n lookup via
the existing interactive.ui labels instead of the literal 'Use /go to apply
confirmed changes.'; call the same ui label used elsewhere (e.g.,
ui.useGoToApply or add that key under interactive.ui/locales if missing) and
pass the result to info() so the message is localized across locales.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d6d613d2-19eb-4794-91df-0ae90bd4510a
📒 Files selected for processing (26)
e2e/specs/cli-workflow-authoring.e2e.tssrc/__tests__/commands-workflow.test.tssrc/__tests__/conversationLoop-builder-go.test.tssrc/__tests__/prompts.test.tssrc/__tests__/slashCommandRegistry.test.tssrc/__tests__/workflow-builder-command.test.tssrc/__tests__/workflow-builder.test.tssrc/app/cli/commands.tssrc/features/interactive/conversationLoop.tssrc/features/interactive/slashCommandRegistry.tssrc/features/workflowAuthoring/builder.tssrc/features/workflowAuthoring/builder/approval.tssrc/features/workflowAuthoring/builder/command.tssrc/features/workflowAuthoring/builder/constants.tssrc/features/workflowAuthoring/builder/files.tssrc/features/workflowAuthoring/builder/manifest.tssrc/features/workflowAuthoring/builder/promptContext.tssrc/features/workflowAuthoring/builder/scope.tssrc/features/workflowAuthoring/builder/snapshot.tssrc/features/workflowAuthoring/builder/types.tssrc/features/workflowAuthoring/builder/validation.tssrc/features/workflowAuthoring/builder/workflowGraph.tssrc/features/workflowAuthoring/index.tssrc/infra/config/loaders/workflowDoctorRefValidator.tssrc/shared/prompts/en/builder_system_prompt.mdsrc/shared/prompts/ja/builder_system_prompt.md
Comment on lines
+364
to
+366
| if (strategy.enableResumeCommand === false) { | ||
| info(ui.retryUnavailable); | ||
| continue; |
There was a problem hiding this comment.
/resume 無効時に誤った案内文を表示しています。
Line 365 で ui.retryUnavailable を流しているため、ユーザーには /retry が無効なのか /resume が無効なのか判別できません。/resume 専用の文言に分けてください。
🤖 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 `@src/features/interactive/conversationLoop.ts` around lines 364 - 366, The
code displays the wrong message when resume is disabled: in the branch checking
strategy.enableResumeCommand === false it currently calls ui.retryUnavailable;
change that call to ui.resumeUnavailable (or add that i18n string if missing) so
the user sees a resume-specific message; update any tests or callers referencing
ui.retryUnavailable in this branch to use ui.resumeUnavailable and keep the
continue behavior in the conversationLoop control flow.
Comment on lines
+54
to
+83
| for (const message of messages) { | ||
| if (message.role === 'assistant') { | ||
| latestAssistantCandidatePaths = findMentionedApprovalCandidatePaths(scope, candidates, message.content); | ||
| continue; | ||
| } | ||
| if (message.role !== 'user') { | ||
| continue; | ||
| } | ||
| for (const segment of splitApprovalTextSegments(message.content)) { | ||
| const mentioned = findMentionedApprovalCandidatePaths(scope, candidates, segment); | ||
| if (isExplicitRejectionLine(segment.trim().toLowerCase())) { | ||
| const rejected = mentioned.size > 0 ? mentioned : latestAssistantCandidatePaths; | ||
| removeApprovedBuilderCandidatePaths(scope, candidates, approvedWorkflowPaths, approvedFacetPaths, rejected); | ||
| continue; | ||
| } | ||
| const hasMentionedCandidate = mentioned.size > 0; | ||
| if (!isExplicitApprovalLine(segment, hasMentionedCandidate)) { | ||
| continue; | ||
| } | ||
| const approved = hasMentionedCandidate ? mentioned : latestAssistantCandidatePaths; | ||
| for (const candidate of candidates) { | ||
| if (!approved.has(candidate.filePath)) { | ||
| continue; | ||
| } | ||
| if (pathIsRejectedInText(scope, candidate.filePath, segment, candidates.map((item) => item.filePath))) { | ||
| removeApprovedBuilderCandidatePath(scope, candidate, approvedWorkflowPaths, approvedFacetPaths); | ||
| continue; | ||
| } | ||
| addApprovedBuilderCandidatePath(scope, candidate, approvedWorkflowPaths, approvedFacetPaths); | ||
| } |
There was a problem hiding this comment.
yes/ok を直前の言及へ自動で結び付けないでください。
Line 73 でパス未明示の承認を latestAssistantCandidatePaths にフォールバックしているため、assistant が単に関連ファイルを説明した直後の yes だけで、その一式が承認済みになります。related edit は明示承認が前提なので、このフォールバックは「直前の assistant 発話が承認依頼だった場合」に限定し、それ以外はユーザー文面内のパス言及を必須にしてください。
💡 修正イメージ
let latestAssistantCandidatePaths = new Set<string>();
+ let latestAssistantApprovalPrompt = false;
...
if (message.role === 'assistant') {
latestAssistantCandidatePaths = findMentionedApprovalCandidatePaths(scope, candidates, message.content);
+ latestAssistantApprovalPrompt = isApprovalRequestMessage(message.content);
continue;
}
...
- const approved = hasMentionedCandidate ? mentioned : latestAssistantCandidatePaths;
+ const approved = hasMentionedCandidate
+ ? mentioned
+ : latestAssistantApprovalPrompt
+ ? latestAssistantCandidatePaths
+ : new Set<string>();isApprovalRequestMessage() 側で、確認依頼の表現だけを許可するのが安全です。
🤖 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 `@src/features/workflowAuthoring/builder/approval.ts` around lines 54 - 83, The
current fallback to latestAssistantCandidatePaths when the user says `yes`/`ok`
causes implicit approvals; change the logic so that when hasMentionedCandidate
is false you only use latestAssistantCandidatePaths if the prior assistant
message was an explicit approval request (use isApprovalRequestMessage(...) on
the assistant message that produced latestAssistantCandidatePaths); otherwise
require explicit path mentions and skip approval. Update the branch around
approved = hasMentionedCandidate ? mentioned : latestAssistantCandidatePaths to
check isApprovalRequestMessage(...) and call removeApprovedBuilderCandidatePaths
/ addApprovedBuilderCandidatePath accordingly.
Comment on lines
+91
to
+99
| async function selectRequiredOption<T extends string>( | ||
| message: string, | ||
| choices: SelectOptionItem<T>[], | ||
| ): Promise<T> { | ||
| const selected = await selectOption<T>(message, choices); | ||
| if (!selected) { | ||
| throw new Error('Workflow builder cancelled before conversation started.'); | ||
| } | ||
| return selected; |
There was a problem hiding this comment.
ウィザード中断を例外で落とさないでください。
selectOption() が未選択を返したときにここで throw すると、開始前にユーザーがキャンセルしただけでも builderWorkflowCommand() 全体が reject されます。対話型 CLI の通常中断として扱い、早期 return で抜けた方が UX が安定します。
🤖 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 `@src/features/workflowAuthoring/builder/command.ts` around lines 91 - 99,
selectRequiredOption currently throws when selectOption returns no selection,
which causes the whole builderWorkflowCommand to reject on simple user cancel;
change selectRequiredOption to stop throwing and instead return a nullable
result (e.g., Promise<T | undefined>) when no selection is made, so callers can
handle an early return/abort gracefully; update the selectRequiredOption
signature and body to return undefined on cancel and then update callers such as
builderWorkflowCommand to check for falsy/undefined and return early instead of
relying on exceptions.
Comment on lines
+132
to
+141
| const parsed = parseBuilderManifestForGo(options.projectDir, options.scope, result.content); | ||
| if (!parsed) { | ||
| return null; | ||
| } | ||
| const { manifest, manifestChanges } = parsed; | ||
| const plannedViolation = findBuilderChangeViolation(options.scope, manifestChanges, approval); | ||
| if (plannedViolation) { | ||
| error(plannedViolation); | ||
| info('Workflow builder did not apply changes. Confirm the target scope and run /go again.'); | ||
| return null; |
There was a problem hiding this comment.
/go の失敗理由を会話履歴にも戻してください。
マニフェスト解析失敗と承認範囲違反は端末出力だけで終了しており、goContext.history に追加されません。このままだと直後に /go を再実行しても、モデル側には前回の失敗理由が渡らず修正ループが切れます。workflow doctor / apply 失敗時と同じく、ここでも buildBuilderValidationFeedback(...) を履歴へ積むべきです。
Also applies to: 213-225
🤖 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 `@src/features/workflowAuthoring/builder/command.ts` around lines 132 - 141,
When parseBuilderManifestForGo(...) returns falsy or
findBuilderChangeViolation(...) returns a plannedViolation, append the
validation feedback into goContext.history (same approach as in workflow
doctor/apply) instead of only logging to the terminal: call
buildBuilderValidationFeedback(...) with the appropriate inputs (e.g.,
options.scope, manifest or manifestChanges, and the error/violation message) and
push the result onto goContext.history before returning null; apply the same
change for the other block referenced around the 213-225 range so both
manifest-parse failures and approval-scope violations record feedback in
history.
Comment on lines
+18
to
+26
| export function listFilesRecursive(rootDir: string, extensions?: string[]): string[] { | ||
| if (!existsSync(rootDir)) { | ||
| return []; | ||
| } | ||
| if (lstatSync(rootDir).isSymbolicLink()) { | ||
| return []; | ||
| } | ||
| const files: string[] = []; | ||
| for (const entry of readdirSync(rootDir)) { |
There was a problem hiding this comment.
ルートが通常ファイルでもここで readdirSync() が落ちます。
existsSync() の後にディレクトリ判定がないので、.takt/workflows などが誤って通常ファイルになっているだけで、ターゲット列挙や在庫生成が ENOTDIR で失敗します。ここは symlink と同様に isDirectory() も先に弾いた方が安全です。
差分案
export function listFilesRecursive(rootDir: string, extensions?: string[]): string[] {
if (!existsSync(rootDir)) {
return [];
}
- if (lstatSync(rootDir).isSymbolicLink()) {
+ const rootStat = lstatSync(rootDir);
+ if (rootStat.isSymbolicLink()) {
+ return [];
+ }
+ if (!rootStat.isDirectory()) {
return [];
}
const files: string[] = [];🤖 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 `@src/features/workflowAuthoring/builder/files.ts` around lines 18 - 26,
listFilesRecursive currently calls readdirSync on rootDir even when rootDir is a
regular file; add a directory check like the existing symlink guard to return []
when lstatSync(rootDir).isDirectory() is false. In other words, in function
listFilesRecursive, after existsSync(rootDir) and before calling
readdirSync(rootDir), call lstatSync(rootDir) and if !isDirectory() return [];
this mirrors the symlink exclusion and prevents ENOTDIR errors from readdirSync.
| ResolvedBuilderScope, | ||
| } from './types.js'; | ||
|
|
||
| const BUILDER_MANIFEST_JSON_BLOCK = /```(?:json)?\s*([\s\S]*?)```/i; |
There was a problem hiding this comment.
任意の最初のコードフェンスを manifest として扱わないでください。
Line 75 は json 指定のない最初のフェンスも剥がすので、assistant が説明用の yaml などを先に返すと、その場で `JSON.parse` が落ちます。`/go` の返答は自由文を含みうるため、ここは ` json ` を優先し、未指定フェンスは「レスポンス全体が単一フェンスのときだけ」許可した方が安全です。
💡 修正案
-const BUILDER_MANIFEST_JSON_BLOCK = /```(?:json)?\s*([\s\S]*?)```/i;
+const BUILDER_MANIFEST_JSON_BLOCK = /```json\s*([\s\S]*?)```/i;
+const BUILDER_MANIFEST_SINGLE_FENCE = /^\s*```(?:json)?\s*([\s\S]*?)```\s*$/i;
...
function extractBuilderManifestJson(content: string): string {
const fenced = BUILDER_MANIFEST_JSON_BLOCK.exec(content);
if (fenced?.[1]) {
return fenced[1].trim();
}
+ const singleFence = BUILDER_MANIFEST_SINGLE_FENCE.exec(content);
+ if (singleFence?.[1]) {
+ return singleFence[1].trim();
+ }
return content.trim();
}Also applies to: 74-79
🤖 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 `@src/features/workflowAuthoring/builder/manifest.ts` at line 21, 現在の
BUILDER_MANIFEST_JSON_BLOCK が最初のコードフェンス（json 指定なし）もマッチしてしまい説明用の ```yaml などで
JSON.parse が失敗するので、まず ```json
を優先して抽出し、未指定フェンスはレスポンス全体が単一フェンスだった場合のみ許可するように修正してください: 定数を
BUILDER_MANIFEST_JSON_BLOCK = /```json\s*([\s\S]*?)```/i とし、別で
BUILDER_MANIFEST_SINGLE_FENCE = /^\s*```(?:json)?\s*([\s\S]*?)```\s*$/i
を定義、extractBuilderManifestJson(content: string) 内でまず BUILDER_MANIFEST_JSON_BLOCK
を試し、マッチしなければ BUILDER_MANIFEST_SINGLE_FENCE を試してマッチ時はそのグループを返し（trim
する）、どちらにもマッチしなければ元の content.trim() を返すようにしてください（関数名 extractBuilderManifestJson
と定数名を参照）。
Comment on lines
+42
to
+48
| export function buildBuilderSystemPrompt( | ||
| lang: 'en' | 'ja', | ||
| context: BuilderPromptContext, | ||
| ): string { | ||
| return loadTemplate('builder_system_prompt', lang, { | ||
| styleGuide: loadStyleGuide(), | ||
| yamlSchema: loadYamlSchema(), |
There was a problem hiding this comment.
英語プロンプトにも日本語の STYLE_GUIDE を注入しています。
buildBuilderSystemPrompt('en', ...) でも loadStyleGuide() が常に getLanguageResourcesDir('ja') を読むため、英語ビルダーが日本語ガイド前提で動きます。lang を引き回して同一言語のガイドを読むようにしないと、英語側の出力品質が不安定です。
💡 修正案
export function buildBuilderSystemPrompt(
lang: 'en' | 'ja',
context: BuilderPromptContext,
): string {
return loadTemplate('builder_system_prompt', lang, {
- styleGuide: loadStyleGuide(),
+ styleGuide: loadStyleGuide(lang),
yamlSchema: loadYamlSchema(),
scopeSummary: formatUntrustedReferenceBlock('Scope summary', context.scopeSummary),
assetInventory: formatUntrustedReferenceBlock('Existing assets', context.assetInventory),
targetContext: formatUntrustedReferenceBlock('Selected target context', context.targetContext),
relatedGraph: formatUntrustedReferenceBlock('Related workflow candidates', context.relatedGraph),
});
}
-function loadStyleGuide(): string {
- const builtinsJaDir = getLanguageResourcesDir('ja');
+function loadStyleGuide(lang: 'en' | 'ja'): string {
+ const styleGuideDir = getLanguageResourcesDir(lang);
const styleGuideFiles = [
'STYLE_GUIDE.md',
'PERSONA_STYLE_GUIDE.md',
'POLICY_STYLE_GUIDE.md',
'KNOWLEDGE_STYLE_GUIDE.md',
'INSTRUCTION_STYLE_GUIDE.md',
'OUTPUT_CONTRACT_STYLE_GUIDE.md',
];
return styleGuideFiles
.map((fileName) => {
- const filePath = join(builtinsJaDir, fileName);
+ const filePath = join(styleGuideDir, fileName);
return existsSync(filePath)
? `## ${fileName}\n${readFileSync(filePath, 'utf-8')}`
: '';
})
.filter((content) => content.length > 0)
.join('\n\n');
}Also applies to: 108-126
🤖 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 `@src/features/workflowAuthoring/builder/promptContext.ts` around lines 42 -
48, The buildBuilderSystemPrompt is injecting the Japanese style guide because
loadStyleGuide() (and similarly loadYamlSchema()) currently always reads
getLanguageResourcesDir('ja'); update buildBuilderSystemPrompt to pass the lang
through to the resource loaders so they load the matching language (e.g., change
calls to loadStyleGuide(lang) and loadYamlSchema(lang) or modify those helpers
to accept a language parameter), and apply the same change to the other builder
functions in the file (the block around lines 108-126) so English prompts
receive English resources and Japanese prompts receive Japanese resources.
Comment on lines
+71
to
+88
| function buildScopeRelatedGraph(scope: ResolvedBuilderScope): string { | ||
| const sections = listBuilderTargetWorkflows(scope).map((workflow) => { | ||
| const analysis = buildRelatedWorkflowAnalysis({ | ||
| scope, | ||
| targetWorkflowPath: workflow.path, | ||
| }); | ||
| const candidateText = formatRelatedCandidates(scope, analysis.candidates); | ||
| const diagnosticText = formatRelatedDiagnostics(analysis.diagnostics); | ||
| return [ | ||
| `## ${formatScopedPath(scope, workflow.path)}`, | ||
| candidateText || 'No related workflow candidates detected.', | ||
| diagnosticText, | ||
| ].join('\n'); | ||
| }); | ||
| return sections.length > 0 | ||
| ? sections.join('\n\n') | ||
| : 'No workflow files were found in the selected scope.'; | ||
| } |
There was a problem hiding this comment.
関連グラフに本文を丸ごと埋め込みすぎて、スコープが大きいとプロンプトが膨らみすぎます。
ここは全 workflow ごとに関連候補を列挙したうえで、各候補の workflow 本文と facet 本文まで展開しています。スコープ内 workflow 数に対してほぼ二乗でトークン量が増えるので、実運用ではコンテキスト超過や応答劣化を起こしやすいです。関連グラフには path/relation/reason だけを載せて、必要な本文は会話中に Read で取りに行かせた方が安定します。
💡 修正案
function formatRelatedCandidates(scope: ResolvedBuilderScope, candidates: RelatedWorkflowCandidate[]): string {
return candidates
.map((candidate) => [
`- ${candidate.relation}: ${formatScopedPath(scope, candidate.workflowPath)}`,
` reason: ${candidate.reason}`,
- indentReferenceBlock(formatScopedReference(scope, 'Related workflow body', candidate.workflowPath)),
- ...formatRelatedCandidateFacets(scope, candidate.workflowPath).map(indentReferenceBlock),
].join('\n'))
.join('\n');
}
-
-function formatRelatedCandidateFacets(scope: ResolvedBuilderScope, workflowPath: string): string[] {
- if (!isScopedReadableFile(scope, workflowPath)) {
- return [];
- }
- const raw = loadRawWorkflow(workflowPath);
- return resolveUsedFacetPaths(scope, raw, workflowPath)
- .filter((facetPath) => existsSync(facetPath))
- .map((facetPath) => formatScopedReference(scope, 'Related referenced facet', facetPath));
-}Also applies to: 165-193
🤖 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 `@src/features/workflowAuthoring/builder/promptContext.ts` around lines 71 -
88, The current buildScopeRelatedGraph function (and related logic used at lines
~165-193) inlines full workflow and facet bodies for every candidate, causing
quadratic token growth; change buildScopeRelatedGraph to only emit per-candidate
metadata (path, relation, reason) by replacing/adjusting the use of
formatRelatedCandidates (or creating a new formatter) so it returns only "path /
relation / reason" entries, leaving out workflow/facet text, and ensure
buildRelatedWorkflowAnalysis consumers still provide candidate identifiers so
the actual bodies can be retrieved later on explicit "Read" requests during the
conversation.
Comment on lines
+31
to
+38
| if (changedFacets.size > 0) { | ||
| for (const workflow of listBuilderTargetWorkflows(options.scope)) { | ||
| const raw = WorkflowConfigRawSchema.parse(parseYamlContent(workflow.path)); | ||
| const usedFacets = resolveUsedFacetPaths(options.scope, raw, workflow.path); | ||
| if (usedFacets.some((facetPath) => changedFacets.has(resolve(facetPath)))) { | ||
| targets.add(resolve(workflow.path)); | ||
| } | ||
| } |
There was a problem hiding this comment.
既存の壊れた workflow 1 件で facet 変更全体が止まります。
Line 33 で scope 内の全 workflow を parse しているので、無関係な既存 YAML/Schema エラーが 1 件あるだけで facet 変更時の検証対象解決が例外終了します。少なくともこの走査は、同ファイルの parseWorkflowForApprovalScope() と同様に「読めない workflow はスキップ」で扱わないと、既存不整合のあるリポジトリで builder が使えません。
💡 修正案
if (changedFacets.size > 0) {
for (const workflow of listBuilderTargetWorkflows(options.scope)) {
- const raw = WorkflowConfigRawSchema.parse(parseYamlContent(workflow.path));
+ const raw = parseWorkflowForApprovalScope(workflow.path);
+ if (!raw) {
+ continue;
+ }
const usedFacets = resolveUsedFacetPaths(options.scope, raw, workflow.path);
if (usedFacets.some((facetPath) => changedFacets.has(resolve(facetPath)))) {
targets.add(resolve(workflow.path));
}
}📝 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
| if (changedFacets.size > 0) { | |
| for (const workflow of listBuilderTargetWorkflows(options.scope)) { | |
| const raw = WorkflowConfigRawSchema.parse(parseYamlContent(workflow.path)); | |
| const usedFacets = resolveUsedFacetPaths(options.scope, raw, workflow.path); | |
| if (usedFacets.some((facetPath) => changedFacets.has(resolve(facetPath)))) { | |
| targets.add(resolve(workflow.path)); | |
| } | |
| } | |
| if (changedFacets.size > 0) { | |
| for (const workflow of listBuilderTargetWorkflows(options.scope)) { | |
| const raw = parseWorkflowForApprovalScope(workflow.path); | |
| if (!raw) { | |
| continue; | |
| } | |
| const usedFacets = resolveUsedFacetPaths(options.scope, raw, workflow.path); | |
| if (usedFacets.some((facetPath) => changedFacets.has(resolve(facetPath)))) { | |
| targets.add(resolve(workflow.path)); | |
| } | |
| } |
🤖 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 `@src/features/workflowAuthoring/builder/validation.ts` around lines 31 - 38,
The loop over listBuilderTargetWorkflows currently calls
WorkflowConfigRawSchema.parse(parseYamlContent(workflow.path)) directly which
throws on any unreadable/invalid YAML and aborts facet-change resolution; wrap
the parse step in a try/catch (or reuse the same “skip unreadable workflow”
logic used by parseWorkflowForApprovalScope) so that if parseYamlContent or
WorkflowConfigRawSchema.parse fails you log or warn and continue to the next
workflow, then only call resolveUsedFacetPaths and potentially add
resolve(workflow.path) to targets for successfully parsed workflows.
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
1 participant
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
背景・動機
ワークフローを作るには、YAML スキーマとファセット分離（persona/policy/knowledge/instruction/output-contract）を理解した上で手書きする必要がある。
takt workflow initは静的雛形を出すだけで設計判断は人間任せ。takt-builderの知識（STYLE_GUIDE、yaml-schema、ファセット分離判断）を 対話する AI（builder） として常駐させ、要件を話すだけで設計・生成・検証できるようにする。workflow名前空間に、doctor（診断する人）と対になる役割名詞builder（組み立てる人）を追加する。コマンド
src/app/cli/commands.tsのworkflowサブコマンドにinit/doctorと並べて登録。設計の中心：ワークフロー中心、ファセットは選ばない
ユーザーはファセットを直接選ばない。エントリは常にワークフローで、「このワークフローでこういうことをしたい」という意図に合わせて builder がファセット（persona/policy/knowledge/instruction/output-contract）を生成・更新する。
基本方針：軽量ウィザード + 会話のハイブリッド
起動直後に 構造的な軸だけ を軽量ウィザード（対話の選択 UI、CLI フラグではない）で確定させ、そこから先は builder AI との自由対話に落とす。ただしターゲットを絞りたくない場合は、ウィザードを最小限にして即対話へ移れる。
フェーズ1：ウィザード（構造軸の確定）
出力先 scope
.takt/~/.takt/builtins/{en,ja}/（TAKT リポジトリ内でのみ表示。builtins/ja/STYLE_GUIDE.mdの存在で自動検出し、検出時のみ候補に出す）ターゲット
（「既存ワークフローを修正」のみ）対象ワークフロー選択
workflows/*.yamlをGlobで一覧表示し、対象を選ぶ。Glob/Readで探索しながら対話で対象を特定する。フェーズ2：会話（設計・生成）
ウィザードで確定した軸（または「絞らない」前提）で、builder AI と自由対話で中身を詰める。
/goで生成内容を確定、/cancelで破棄。関連ワークフロー・ファセットの波及編集（確認フロー）
ワークフローを1つ指定しても、builder はそれと 関連するファセットや類似ワークフローを探索 し、見つかれば 「これも編集しますか？」とユーザーに問いかけ、OK が出たものだけ編集 する。勝手に広げない。
workflow_call:で呼ぶサブワークフロー／対象を呼ぶ親ワークフローreview-*系）workflowDoctorRefValidator（ファセット参照・未使用検出）とworkflow_call契約検証の仕組みを利用して関連グラフを構築する。ユーザーディレクトリの理解
builder には対象 scope の既存資産を把握させる。
workflows/・facets/・config.yamlを読み、再利用可能な既存資産リストとワークフロー間の関連グラフを systemPrompt に注入（assistantInitFiles.ts/runSessionReader.tsと同じ流儀）。allowedToolsにRead/Glob/Grepを許可し、対話中に自分で深掘りできるようにする。builder の知識・指示はプロンプトテンプレートで持つ
builder の systemPrompt は ファセットではなくプロンプトテンプレート として実装する。
src/shared/prompts/{en,ja}/builder_system_prompt.md（仮）を新設し、loadTemplate('builder_system_prompt', lang, vars)で読む（perform_agent_system_prompt.mdなどと同じ仕組み）。yaml-schema（TAKT 本体builtins/{lang}/の Single Source of Truth）はテンプレート変数として注入し、重複を持たない。builtin（TAKT 開発者）モードの固有挙動
scope =
builtins/のときだけ en / ja の両言語を生成・同期 する。片方で作ってもう一方へミラー（翻訳）。project / global には無い分岐。生成と検証
/goで生成内容を確定 → ワークフロー YAML・関連ファセット・（OK の出た）波及先をWrite。inspectWorkflowFile()（workflow doctorの検証エンジン）を編集対象すべてに対して自動実行し、スキーマ・グラフ到達性・ファセット参照を検証。エラーは対話に差し戻して修正させる。実装スケッチ
src/features/interactive/conversationLoop.ts::runConversationLoop()を再利用。strategy（systemPrompt =loadTemplate('builder_system_prompt', ...)/ allowedTools / transformPrompt / introMessage）を builder 用に差し替える。workflowDoctorRefValidatorとworkflow_call契約検証を流用。aiCaller.ts::callAIWithRetry()、セッション初期化はsessionInitialization.tsを流用。/goフックを「ファイル生成 → Write → doctor 検証」に差し替え。commands.ts:スコープ外
takt runの役割）。受け入れ条件
takt workflow builderを引数・フラグなしで起動できる。src/shared/prompts/{en,ja}/のテンプレートとして実装され、STYLE_GUIDE /yaml-schemaを変数注入している。workflow doctorの検証を通る。Execution Report
Workflow
takt-defaultcompleted successfully.Closes #777
Summary by CodeRabbit
workflow builderCLI command for interactive workflow creation and modification./gocommand support in conversation mode to generate workflow changes.handleGohook,enableResumeCommand, anddisableDirectExecuteCommandsoptions to customize command availability and execution behavior.