diff --git a/.github/workflows/smoke-copilot-sub-agents.lock.yml b/.github/workflows/smoke-copilot-sub-agents.lock.yml index a6bc514565d..a830e9e997e 100644 --- a/.github/workflows/smoke-copilot-sub-agents.lock.yml +++ b/.github/workflows/smoke-copilot-sub-agents.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"8df3b522d89a94ff42e06b6efbfd90673728ba6874ddb4f582ab5f8ad1f67f48","body_hash":"ead03f5b55d16621f4474fb57c28643abfea40c6218edbb0ff309b329c4d4498","strict":true,"agent_id":"copilot","agent_model":"gpt-5.3-codex","engine_versions":{"copilot":"1.0.73","copilot-sdk":"1.0.7"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"a36af2905338f72270a86805d6b21b53e69c06c6f7ce5a10b908bc04845f5179","body_hash":"66dd32a20bb7d4a839aca20b4c9729240d2574e69a5f13f3d4aaa484472974a4","strict":true,"agent_id":"copilot","agent_model":"gpt-5.3-codex","engine_versions":{"copilot":"1.0.73","copilot-sdk":"1.0.7"}} # gh-aw-manifest: {"version":1,"secrets":["COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.38","digest":"sha256:cb928eb62d9139a013c2d278dab19af232d35a2d83dca71a3d98eb431f786243","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.38@sha256:cb928eb62d9139a013c2d278dab19af232d35a2d83dca71a3d98eb431f786243"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.38","digest":"sha256:cd6145620d96acee46e1ede25180a13aa36002467e663db0caa453a8bc8eb60c","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.38@sha256:cd6145620d96acee46e1ede25180a13aa36002467e663db0caa453a8bc8eb60c"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.38","digest":"sha256:6c19094d95aad5f9f128ad5e583f0f2b894b158aa66c3b86dd9bcc90970a2917","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.38@sha256:6c19094d95aad5f9f128ad5e583f0f2b894b158aa66c3b86dd9bcc90970a2917"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.3","digest":"sha256:3c744710ea275cd5ee65db92a1099e0d980754bd9fafda9ce67704c67004dc83","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.3@sha256:3c744710ea275cd5ee65db92a1099e0d980754bd9fafda9ce67704c67004dc83"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -84,6 +84,7 @@ jobs: daily_ai_credits_threshold: ${{ steps.daily-effective-workflow-guardrail.outputs.daily_ai_credits_threshold || '' }} daily_ai_credits_total_effective_tokens: ${{ steps.daily-effective-workflow-guardrail.outputs.daily_ai_credits_total_effective_tokens || '' }} engine_id: ${{ steps.generate_aw_info.outputs.engine_id }} + experiments: ${{ steps.pick-experiment.outputs.experiments }} lockdown_check_failed: ${{ steps.generate_aw_info.outputs.lockdown_check_failed == 'true' }} model: ${{ steps.generate_aw_info.outputs.model }} oauth_token_check_failed: ${{ steps.check-oauth-tokens.outputs.oauth_token_check_failed == 'true' }} @@ -91,6 +92,7 @@ jobs: setup-span-id: ${{ steps.setup.outputs.span-id }} setup-trace-id: ${{ steps.setup.outputs.trace-id }} stale_lock_file_failed: ${{ steps.check-lock-file.outputs.stale_lock_file_failed == 'true' }} + sub_agent_strategy: ${{ steps.pick-experiment.outputs.sub_agent_strategy }} steps: - name: Checkout actions folder uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -228,10 +230,45 @@ jobs: - name: Log runtime features if: ${{ contains(toJSON(vars), '"GH_AW_RUNTIME_FEATURES":') }} run: bash "${RUNNER_TEMP}/gh-aw/actions/log_runtime_features_summary.sh" + - name: Restore experiment state from git + id: restore-experiment-state + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + GH_AW_EXPERIMENT_STATE_FILE: /tmp/gh-aw/experiments/state.json + GH_AW_EXPERIMENT_STATE_DIR: /tmp/gh-aw/experiments + GH_AW_EXPERIMENT_BRANCH: experiments/smokecopilotsubagents + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io, getOctokit); + const { main } = require('${{ runner.temp }}/gh-aw/actions/load_experiment_state_from_repo.cjs'); + await main(); + - name: Pick experiment variants + id: pick-experiment + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + GH_AW_EXPERIMENT_SPEC: '{"sub_agent_strategy":{"variants":["inline_strict","delegated_sequential","single_agent_control"],"description":"Measure whether inline sub-agent orchestration is the best reliability/cost tradeoff for a model-identity smoke test.","hypothesis":"H0: no change in pass_rate between inline_strict and delegated_sequential. H1: delegated_sequential improves pass_rate by \u003e= 0.15 absolute versus inline_strict. Note: single_agent_control is a synthetic negative baseline (always FAIL by design) excluded from H1 comparisons.","metric":"pass_rate","secondary_metrics":["run_duration_seconds","output_validity_rate"],"guardrail_metrics":[{"name":"empty_output_rate","direction":"min","threshold":"0.01"},{"name":"false_pass_rate","direction":"min","threshold":"0.05"}],"min_samples":30,"weight":[34,33,33],"issue":47551,"start_date":"2026-07-23","end_date":"2026-10-23"}}' + GH_AW_EXPERIMENT_STATE_FILE: /tmp/gh-aw/experiments/state.json + GH_AW_EXPERIMENT_STATE_DIR: /tmp/gh-aw/experiments + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io, getOctokit); + const { main } = require('${{ runner.temp }}/gh-aw/actions/pick_experiment.cjs'); + await main(); + - name: Upload experiment artifact + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: smokecopilotsubagents-experiment + path: /tmp/gh-aw/experiments + if-no-files-found: ignore + retention-days: 30 - name: Create prompt with built-in context env: GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt GH_AW_SAFE_OUTPUTS: ${{ runner.temp }}/gh-aw/safeoutputs/outputs.jsonl + GH_AW_EXPERIMENTS_SUB_AGENT_STRATEGY: ${{ steps.pick-experiment.outputs.sub_agent_strategy }} GH_AW_EXPR_1A3A194A: ${{ github.event.discussion.number || (fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_type == 'discussion' && fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_number) }} GH_AW_EXPR_463A214A: ${{ github.event.pull_request.number || (fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_type == 'pull_request' && fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_number) }} GH_AW_EXPR_802A9F6A: ${{ github.event.issue.number || (fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_type == 'issue' && fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_number) }} @@ -302,6 +339,7 @@ jobs: GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} GH_AW_GITHUB_SERVER_URL: ${{ github.server_url }} + GH_AW_EXPERIMENTS_SUB_AGENT_STRATEGY: ${{ steps.pick-experiment.outputs.sub_agent_strategy }} with: script: | const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); @@ -312,6 +350,7 @@ jobs: uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + GH_AW_EXPERIMENTS_SUB_AGENT_STRATEGY: ${{ steps.pick-experiment.outputs.sub_agent_strategy }} GH_AW_EXPR_1A3A194A: ${{ github.event.discussion.number || (fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_type == 'discussion' && fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_number) }} GH_AW_EXPR_463A214A: ${{ github.event.pull_request.number || (fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_type == 'pull_request' && fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_number) }} GH_AW_EXPR_802A9F6A: ${{ github.event.issue.number || (fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_type == 'issue' && fromJSON(github.event.inputs.aw_context || github.event.client_payload.aw_context || '{}').item_number) }} @@ -333,6 +372,7 @@ jobs: return await substitutePlaceholders({ file: process.env.GH_AW_PROMPT, substitutions: { + GH_AW_EXPERIMENTS_SUB_AGENT_STRATEGY: process.env.GH_AW_EXPERIMENTS_SUB_AGENT_STRATEGY, GH_AW_EXPR_1A3A194A: process.env.GH_AW_EXPR_1A3A194A, GH_AW_EXPR_463A214A: process.env.GH_AW_EXPR_463A214A, GH_AW_EXPR_802A9F6A: process.env.GH_AW_EXPR_802A9F6A, @@ -991,6 +1031,7 @@ jobs: - activation - agent - detection + - push_experiments_state - safe_outputs if: > always() && (needs.agent.result != 'skipped' || needs.activation.outputs.lockdown_check_failed == 'true' || @@ -1307,6 +1348,12 @@ jobs: mkdir -p /tmp/gh-aw/ find "/tmp/gh-aw/" -type f -print echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/agent_output.json" >> "$GITHUB_OUTPUT" + - name: Download experiment artifact + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: smokecopilotsubagents-experiment + path: /tmp/gh-aw/experiments/ - name: Checkout repository for patch context if: needs.agent.outputs.has_patch == 'true' uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -1508,6 +1555,81 @@ jobs: } } + push_experiments_state: + needs: activation + if: always() && (!cancelled()) && needs.activation.result == 'success' + runs-on: ubuntu-slim + permissions: + contents: write + env: + GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} + steps: + - name: Checkout actions folder + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + repository: github/gh-aw + sparse-checkout: | + actions + clean: false + persist-credentials: false + - name: Setup Scripts + id: setup + uses: ./actions/setup + with: + destination: ${{ runner.temp }}/gh-aw/actions + job-name: ${{ github.job }} + trace-id: ${{ needs.activation.outputs.setup-trace-id }} + parent-span-id: ${{ needs.activation.outputs.setup-parent-span-id || needs.activation.outputs.setup-span-id }} + env: + GH_AW_SETUP_WORKFLOW_NAME: "Smoke Copilot Sub Agents" + GH_AW_CURRENT_WORKFLOW_REF: ${{ github.repository }}/.github/workflows/smoke-copilot-sub-agents.lock.yml@${{ github.ref }} + GH_AW_INFO_VERSION: "1.0.73" + GH_AW_INFO_AWF_VERSION: "v0.27.38" + GH_AW_INFO_ENGINE_ID: "copilot" + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + sparse-checkout: . + - name: Configure Git credentials + env: + GITHUB_REPOSITORY: ${{ github.repository }} + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_TOKEN: ${{ github.token }} + run: bash "${RUNNER_TEMP}/gh-aw/actions/configure_git_credentials.sh" + - name: Download experiment artifact + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + continue-on-error: true + with: + name: smokecopilotsubagents-experiment + path: /tmp/gh-aw/experiments + - name: Push experiment state to git + id: push_experiments_state + if: always() + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + GH_TOKEN: ${{ github.token }} + GITHUB_RUN_ID: ${{ github.run_id }} + GITHUB_SERVER_URL: ${{ github.server_url }} + GH_AW_EXPERIMENT_STATE_DIR: /tmp/gh-aw/experiments + GH_AW_EXPERIMENT_BRANCH: experiments/smokecopilotsubagents + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io, getOctokit); + const { main } = require('${{ runner.temp }}/gh-aw/actions/push_experiment_state.cjs'); + await main(); + - name: Restore actions folder + if: always() + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + repository: github/gh-aw + sparse-checkout: | + actions/setup + sparse-checkout-cone-mode: true + clean: false + persist-credentials: false + safe_outputs: needs: - activation diff --git a/.github/workflows/smoke-copilot-sub-agents.md b/.github/workflows/smoke-copilot-sub-agents.md index 46b8fc535e8..6be47d6264d 100644 --- a/.github/workflows/smoke-copilot-sub-agents.md +++ b/.github/workflows/smoke-copilot-sub-agents.md @@ -16,6 +16,25 @@ engine: id: copilot copilot-sdk: true bare: true +experiments: + sub_agent_strategy: + variants: [inline_strict, delegated_sequential, single_agent_control] + description: "Measure whether inline sub-agent orchestration is the best reliability/cost tradeoff for a model-identity smoke test." + hypothesis: "H0: no change in pass_rate between inline_strict and delegated_sequential. H1: delegated_sequential improves pass_rate by >= 0.15 absolute versus inline_strict. Note: single_agent_control is a synthetic negative baseline (always FAIL by design) excluded from H1 comparisons." + metric: pass_rate + secondary_metrics: [run_duration_seconds, output_validity_rate] + guardrail_metrics: + - name: empty_output_rate + direction: min + threshold: 0.01 + - name: false_pass_rate + direction: min + threshold: 0.05 + min_samples: 30 + weight: [34, 33, 33] + start_date: "2026-07-23" + end_date: "2026-10-23" + issue: 47551 safe-outputs: create-issue: expires: 2h @@ -34,19 +53,39 @@ features: ## Tasks +{{#if experiments.sub_agent_strategy == 'single_agent_control' }} +1. Do not call any sub-agent. +2. Produce the issue in the same format, but mark each agent as `not_invoked`. +3. Set overall status to FAIL. +4. Do not use unnecessary tool calls. +{{/if}} +{{#if experiments.sub_agent_strategy == 'delegated_sequential' }} 1. Call `haiku-whoami`, `mini-whoami`, and `nano-whoami` exactly once each. 2. Ask each sub-agent exactly this question: `who am i?` 3. Check the exact responses: - `haiku-whoami` → `claude-haiku-4.5` - `mini-whoami` → `gpt-5-mini` - `nano-whoami` → `gpt-5-nano` -4. Do not use any other agent or any unnecessary tool calls. +4. Execute and validate each sub-agent one at a time before moving to the next. +5. Do not use any other agent or any unnecessary tool calls. +{{/if}} +{{#if experiments.sub_agent_strategy == 'inline_strict' }} +1. Call `haiku-whoami`, `mini-whoami`, and `nano-whoami` exactly once each. +2. Ask each sub-agent exactly this question: `who am i?` +3. Check the exact responses: + - `haiku-whoami` → `claude-haiku-4.5` + - `mini-whoami` → `gpt-5-mini` + - `nano-whoami` → `gpt-5-nano` +4. Validate all three results and keep the current inline orchestration behavior. +5. Do not use any other agent or any unnecessary tool calls. +{{/if}} ## Output Always create an issue titled **"Smoke Test: Copilot Sub Agents - ${{ github.run_id }}"** with: - One line per sub-agent showing expected value, actual value, and ✅/❌ - Overall status: PASS only if all three exact matches succeed, otherwise FAIL +- Experiment variant: `{{ experiments.sub_agent_strategy }}` - Run URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} ## agent: `haiku-whoami` diff --git a/docs/adr/47557-ab-experiment-infrastructure-smoke-copilot-sub-agents.md b/docs/adr/47557-ab-experiment-infrastructure-smoke-copilot-sub-agents.md new file mode 100644 index 00000000000..e02bc5d3646 --- /dev/null +++ b/docs/adr/47557-ab-experiment-infrastructure-smoke-copilot-sub-agents.md @@ -0,0 +1,55 @@ +# ADR-47557: Add A/B Experiment Infrastructure to smoke-copilot-sub-agents Workflow + +**Date**: 2026-07-23 +**Status**: Draft +**Deciders**: pelikhan, ab-testing-advisor (automated) + +--- + +### Context + +The `smoke-copilot-sub-agents` workflow has shown reliability instability — 8 of the last 10 visible runs failed. The workflow calls three fixed inline sub-agents (`haiku-whoami`, `mini-whoami`, `nano-whoami`) and verifies exact identity strings, but it is unclear whether the current inline orchestration strategy is optimal or whether a different strategy would improve pass rate. To answer this question with statistical rigor, the team needs a mechanism to route each workflow run to one of three orchestration variants (`inline_strict`, `delegated_sequential`, `single_agent_control`), accumulate variant-level metrics automatically over ≥30 runs each, and propagate the selected variant into prompt logic at runtime. + +The `gh-aw` compiler's handlebars template system already supports `experiments.` variable substitution, but had no validation to prevent authors from writing fragile inline conditional syntax (e.g., `4. {{#if cond}}A{{else}}B{{/if}}` on one line), which the experiment prompt draft surfaced as a latent authoring risk. + +### Decision + +We will add experiment metadata to the workflow frontmatter under `experiments.sub_agent_strategy`, implement variant-conditional prompt blocks using the existing handlebars template system with separators placed on their own lines, and persist per-run assignment state to a dedicated git branch (`experiments/smokecopilotsubagents`) via GitHub Actions artifacts. We will also add a compiler validation rule (`detectMidlineTemplateSeparators`) that warns — but does not hard-error — when template separators appear mid-line in workflow markdown. + +The variant assignment is performed at the start of the `activation` job via a `pick-experiment` step that reads prior state from git, selects the next variant weighted round-robin (34/33/33), and writes the result to an artifact uploaded per run. A dedicated `push_experiments_state` job (running in parallel with the agent) commits accumulated state back to the experiments branch after each run. + +### Alternatives Considered + +#### Alternative 1: External A/B Testing Service (e.g., LaunchDarkly, Unleash) + +Use an external feature-flag or experimentation platform to assign variants at runtime. This would provide rich dashboards and statistical analysis out of the box, but adds an external service dependency, requires authentication secrets, and introduces a network call in the critical activation path. It is also disproportionate for a single-workflow experiment with a 30-run sample target. Rejected due to complexity and dependency cost. + +#### Alternative 2: Static Variant Hardcoding with Manual PR Rotation + +Hardcode a single variant per run and rotate it via separate PRs for each variant window. This avoids runtime infrastructure but requires human intervention per variant switch, prevents concurrent multi-variant collection, and cannot produce reliable statistical samples automatically when run frequency is low. Rejected because it defeats the purpose of automated experimentation. + +#### Alternative 3: Separate Workflow Files per Variant + +Create three copies of the workflow, one per variant, each running independently. This isolates variants at the file level but triples maintenance burden, makes it harder to ensure identical conditions across variants (other than the orchestration dimension), and would require three separate scheduling configurations. Rejected due to maintenance overhead and risk of configuration drift between variants. + +### Consequences + +#### Positive +- Statistically sound experiment data collected automatically across ≥30 runs per variant without human intervention +- Variants are self-documenting in workflow frontmatter; hypothesis and guardrails are co-located with the implementation +- Compiler warning (`detectMidlineTemplateSeparators`) prevents future authors from writing fragile inline conditional blocks; surfaces the issue at compile time rather than at runtime +- State persists in git with no external service dependency; artifact-based state survives across workflow runs + +#### Negative +- The `experiments/smokecopilotsubagents` branch is a long-lived branch that must be deleted manually after the experiment concludes (~90 days); no automatic cleanup is implemented +- The `push_experiments_state` job runs unconditionally after every `activation` success, adding a small but non-zero CI overhead (checkout + git push) even between experiment campaigns +- The `single_agent_control` variant intentionally always produces FAIL status, which artificially depresses the overall workflow success rate in monitoring dashboards during the experiment window + +#### Neutral +- Experiment state is stored as JSON in a git branch rather than a database; this is readable but not queryable without downloading the branch +- The compiler warning increments `WarningCount` but is not a hard error; existing workflows with inline separators will produce warning noise until authors migrate them +- The experiment infrastructure (artifact upload/download, state branch) is workflow-specific; generalizing it to other workflows would require the same boilerplate to be added to each + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/workflow/compiler_orchestrator_frontmatter.go b/pkg/workflow/compiler_orchestrator_frontmatter.go index 5fe77deb281..7e1f60aa520 100644 --- a/pkg/workflow/compiler_orchestrator_frontmatter.go +++ b/pkg/workflow/compiler_orchestrator_frontmatter.go @@ -250,7 +250,15 @@ func (c *Compiler) parseFrontmatterSection(markdownPath string) (*frontmatterPar // the compiler converts double quotes to single quotes automatically — but authors // should fix the source to use single quotes to keep it consistent with the output. for _, w := range detectDoubleQuotedExperimentComparisons(result.Markdown) { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(w)) + fmt.Fprintln(os.Stderr, formatCompilerMessage(cleanPath, "warning", w)) + c.IncrementWarningCount() + } + + // Warn when template separators are embedded in the middle of a line. + // Keeping separators on their own lines improves compatibility with the + // template renderer and avoids brittle inline condition blocks. + for _, w := range detectMidlineTemplateSeparators(result.Markdown) { + fmt.Fprintln(os.Stderr, formatCompilerMessage(cleanPath, "warning", w)) c.IncrementWarningCount() } diff --git a/pkg/workflow/template_include_validation_test.go b/pkg/workflow/template_include_validation_test.go index e7282020420..d520e9316ad 100644 --- a/pkg/workflow/template_include_validation_test.go +++ b/pkg/workflow/template_include_validation_test.go @@ -608,3 +608,86 @@ func TestDetectDoubleQuotedExperimentComparisons(t *testing.T) { }) } } + +func TestDetectMidlineTemplateSeparators(t *testing.T) { + tests := []struct { + name string + input string + wantWarnings int + wantContains string + }{ + { + name: "separators on own lines - no warning", + input: `{{#if experiments.mode == 'a'}} +content +{{#else}} +other +{{/if}}`, + wantWarnings: 0, + }, + { + name: "inline if/else/endif - warning", + input: `4. {{#if experiments.mode == 'a'}}A{{#else}}B{{/if}}`, + wantWarnings: 3, + wantContains: "line 1", + }, + { + name: "elseif separator in middle of line - warning", + input: `{{#if experiments.mode == 'a'}} +{{#elseif experiments.mode == 'b'}} branch-b +{{/if}}`, + wantWarnings: 1, + wantContains: "#elseif", + }, + { + name: "mixed lines with one offending separator", + input: `{{#if experiments.mode == 'a'}} +content {{#else}} +{{/if}}`, + wantWarnings: 1, + wantContains: "{{#else}}", + }, + { + name: "condition with nested github expression - own line no warning", + input: `{{#if ${{ github.actor }} != ''}} +content +{{/if}}`, + wantWarnings: 0, + }, + { + name: "condition with nested github expression mid-line - warning", + input: `prefix {{#if ${{ github.actor }} != ''}} suffix`, + wantWarnings: 1, + wantContains: "mid-line", + }, + { + name: "endif closing form on own line - no warning", + input: `{{#if experiments.mode == 'a'}} +content +{{#endif}}`, + wantWarnings: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + warnings := detectMidlineTemplateSeparators(tt.input) + if len(warnings) != tt.wantWarnings { + t.Errorf("detectMidlineTemplateSeparators() = %d warning(s), want %d; got: %v", + len(warnings), tt.wantWarnings, warnings) + } + if tt.wantContains != "" { + found := false + for _, w := range warnings { + if strings.Contains(w, tt.wantContains) { + found = true + break + } + } + if !found { + t.Errorf("detectMidlineTemplateSeparators() warnings %v do not contain %q", warnings, tt.wantContains) + } + } + }) + } +} diff --git a/pkg/workflow/template_validation.go b/pkg/workflow/template_validation.go index 74c0d01a092..c204b11d945 100644 --- a/pkg/workflow/template_validation.go +++ b/pkg/workflow/template_validation.go @@ -58,6 +58,23 @@ var ( // expression syntax only supports single-quoted string literals, so double quotes must be // replaced with single quotes before the expression reaches the lock file. experimentDoubleQuotePattern = regexp.MustCompile(`experiments\.[a-zA-Z_][a-zA-Z0-9_]*\s*(?:!==?|===?)\s*"[^"]*"`) + + // templateSeparatorPattern matches template block separator tags used by the markdown + // renderer (render_template.cjs, template_branch.cjs). We warn when these separators + // appear mid-line because inline placement is fragile and can lead to hard-to-debug + // rendering behavior. + // + // Recognised forms (mirroring the renderer's grammar): + // {{#if }} — opening tag + // {{#elseif }} (and variants: #else-if, #else_if, elseif, else-if, else_if) + // {{#else}} — else branch (canonical; {{else}} without # is NOT rendered) + // {{#endif}} — primary closing tag + // {{/if}} — alternate closing tag + // + // The expression group reuses the same nested-expression grammar as TemplateIfPattern and + // TemplateElseIfPattern so that conditions containing ${{ ... }} sub-expressions are + // matched correctly (plain [^}]+ would stop at the inner brace). + templateSeparatorPattern = regexp.MustCompile(`\{\{(?:#if\s+(?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])*\s*|#?else[-_]?if\s+(?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])*\s*|#else\s*|#endif\s*|/if\s*)\}\}`) ) // validateNoIncludesInTemplateRegions checks that import directives @@ -190,3 +207,42 @@ func detectDoubleQuotedExperimentComparisons(markdown string) []string { templateValidationLog.Printf("Found %d double-quoted experiment comparison(s)", len(warnings)) return warnings } + +// detectMidlineTemplateSeparators scans template block separator tags and returns warnings +// for lines where separators are embedded alongside other text. +// +// Example (warn): "4. {{#if cond}}A{{else}}B{{/if}}" +// Example (ok): "{{#if cond}}" on its own line. +func detectMidlineTemplateSeparators(markdown string) []string { + templateValidationLog.Print("Checking for mid-line template separators") + + if !strings.Contains(markdown, "{{") { + return nil + } + + lines := strings.Split(markdown, "\n") + var warnings []string + + for i, line := range lines { + matches := templateSeparatorPattern.FindAllStringIndex(line, -1) + if len(matches) == 0 { + continue + } + + for _, match := range matches { + separator := line[match[0]:match[1]] + before := strings.TrimSpace(line[:match[0]]) + after := strings.TrimSpace(line[match[1]:]) + if before == "" && after == "" { + continue + } + warnings = append(warnings, fmt.Sprintf( + "template separator appears mid-line at line %d: %q — place template separators on their own lines", + i+1, separator, + )) + } + } + + templateValidationLog.Printf("Found %d mid-line template separator warning(s)", len(warnings)) + return warnings +}