[ab-advisor] A/B experiment: prefetch_strategy for daily-safe-output-optimizer#38096
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…sages Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new A/B experiment (log_fetch_strategy) to the daily-safe-output-optimizer workflow and introduces conditional import support so imports can be guarded by runtime experiment conditions, affecting both generated workflow steps and prompt content.
Changes:
- Add
if:support to workflow import specs and propagate it into prompt assembly and step generation. - Inject step-level
if:guards for steps originating from conditional imports by rewritingexperiments.<name>→needs.activation.outputs.<name>. - Update
daily-safe-output-optimizerto run an eager-vs-lazy log fetching experiment and persist experiment state.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_yaml.go | Wraps prompt imports/runtime-import macros in {{#if ...}} blocks when an import has an if: condition. |
| pkg/parser/schemas/main_workflow_schema.json | Extends schema to allow if on import object variants (path/uses and aw subfield). |
| pkg/parser/import_remote.go | Adds ifCondition field to queued import items for conditional import propagation. |
| pkg/parser/import_processor.go | Extends ImportSpec and PromptImportEntry with an If condition field. |
| pkg/parser/import_field_extractor.go | Injects conditional if: guards into imported YAML step lists and adds helpers for condition rewriting. |
| pkg/parser/import_field_extractor_test.go | Updates tests for the new extractConfigFields signature. |
| pkg/parser/import_bfs.go | Parses top-level import if: field and threads it into queue items. |
| .github/workflows/daily-safe-output-optimizer.md | Adds experiment config, conditional import of log-fetcher, and prompt branching for eager vs lazy behavior. |
| .github/workflows/daily-safe-output-optimizer.lock.yml | Regenerates the compiled workflow to include experiment selection/state steps and conditional log-download step. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 6
| for i := range steps { | ||
| if _, hasIf := steps[i]["if"]; !hasIf { | ||
| steps[i]["if"] = stepCondition | ||
| } | ||
| } |
| func enqueueImportPath(state *importBFSState, importPath, fullPath, sectionName, baseDir string, inputs map[string]any, ifCondition string, origin *remoteImportOrigin) error { | ||
| if !state.visited[fullPath] { | ||
| state.visited[fullPath] = true | ||
| state.visitedInputs[fullPath] = inputs | ||
| state.queue = append(state.queue, importQueueItem{ | ||
| importPath: importPath, fullPath: fullPath, sectionName: sectionName, baseDir: baseDir, inputs: inputs, remoteOrigin: origin, | ||
| importPath: importPath, fullPath: fullPath, sectionName: sectionName, baseDir: baseDir, inputs: inputs, ifCondition: ifCondition, remoteOrigin: origin, | ||
| }) |
| if entry.If != "" { | ||
| userPromptChunks = append(userPromptChunks, fmt.Sprintf("{{#if %s}}", entry.If)) | ||
| } | ||
| chunks, exprMaps := extractPromptChunksFromMarkdown(cleaned) | ||
| userPromptChunks = append(userPromptChunks, chunks...) | ||
| expressionMappings = append(expressionMappings, exprMaps...) | ||
| if entry.If != "" { | ||
| userPromptChunks = append(userPromptChunks, "{{/if}}") | ||
| } |
| {{#if experiments.log_fetch_strategy == 'eager'}} | ||
| Logs have been pre-downloaded to `/tmp/gh-aw/aw-mcp/logs/` by the setup step. Use this pre-fetched data directly — do **not** call the `logs` MCP tool. | ||
|
|
| func applyIfConditionToStepsYAML(stepsYAML string, ifCondition string) string { | ||
| if ifCondition == "" { | ||
| return stepsYAML | ||
| } | ||
| stepCondition := transformImportConditionForStep(ifCondition) | ||
| var steps []map[string]any | ||
| if err := yaml.UnmarshalWithOptions([]byte(stepsYAML), &steps, yaml.UseOrderedMap()); err != nil { | ||
| parserLog.Printf("Warning: failed to parse steps YAML for if-condition injection (condition=%q), using original: %v", ifCondition, err) | ||
| return stepsYAML | ||
| } | ||
| for i := range steps { | ||
| if _, hasIf := steps[i]["if"]; !hasIf { | ||
| steps[i]["if"] = stepCondition | ||
| } | ||
| } | ||
| out, err := yaml.Marshal(steps) | ||
| if err != nil { | ||
| parserLog.Printf("Warning: failed to marshal steps YAML after if-condition injection (condition=%q), using original: %v", ifCondition, err) | ||
| return stepsYAML | ||
| } | ||
| return strings.TrimSpace(string(out)) | ||
| } |
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Ran pr-finisher pass and pushed fixes in 14b7577. Addressed conditional import gating/propagation, strict |
Adds the
log_fetch_strategyA/B experiment todaily-safe-output-optimizer, comparing eager pre-fetching (setup step downloads logs before agent starts) against lazy on-demand fetching (agent calls MCPlogstool at runtime). Also introduces the infrastructure required to supportif:-conditioned imports.New infrastructure: conditional import support
ImportSpecandPromptImportEntrygain anIf stringfield. The condition propagates through the full import pipeline and produces two effects at compile time:Steps — each step from the conditional import gets
if: needs.activation.outputs.<name> == '<value>'injected (viaapplyIfConditionToStepsYAML+experimentConditionRegex).Prompt — the
{{#runtime-import}}in the lock file is wrapped in{{#if experiments.<name> == '<value>'}}…{{/if}}, whichinterpolate_prompt.cjsevaluates at runtime (runtime-imports are expanded before{{#if}}blocks).Usage in workflow frontmatter:
JSON schema updated to allow
ifon all four import object variants (top-levelpath/usesandaw-subfieldpath/uses).Workflow changes (
daily-safe-output-optimizer.md)shared/aw-logs-24h-fetch.mdimport is now conditional oneagervariant; the download step in the lock file getsif: needs.activation.outputs.log_fetch_strategy == 'eager'agentic-workflows:tool andcache-memory: truedirectly to the main workflow so thelazyvariant retains full MCP accessexperiments: log_fetch_strategyblock (variants:eager/lazy, 50/50, Mann-Whitney U,min_samples: 20, guardrail:issue_creation_success_rate >= 0.80){{#if experiments.log_fetch_strategy == 'eager'}}…{{else}}…{{/if}}: eager instructs the agent to read from/tmp/gh-aw/aw-mcp/logs/directly; lazy retains the existing MCPlogstool call instruction