Skip to content

Appeng 4932/ab eval flow templates - #6

Merged
GuyZivRH merged 7 commits into
mainfrom
APPENG-4932/ab-eval-flow-templates
Apr 14, 2026
Merged

Appeng 4932/ab eval flow templates#6
GuyZivRH merged 7 commits into
mainfrom
APPENG-4932/ab-eval-flow-templates

Conversation

@GuyZivRH

@GuyZivRH GuyZivRH commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Completes the A/B generalisation started in PR #5 by eliminating the last skilled/unskilled remnants in templates, Tekton task definitions, and documentation.

Changes

  • Commit 1 — Unify Dockerfile.skilled.j2 + Dockerfile.unskilled.j2 into a single Dockerfile.j2 driven by copy_pairs loop; update task.toml.j2 to key off skills_dir value instead of variant name; remove variant-to-template bridge code from scaffold.py
  • Commit 2 — Rename Tekton scaffold task results (skilled-task-dirtreatment-task-dir, unskilled-task-dircontrol-task-dir) and align step variables/paths
  • Commit 3 — Update terminology across implementation_plan.md, README.md, ab_testing_generalization_plan.md; add workstreams_roadmap.md
  • Commit 4 (review fix) — Remove dead has_docs context key (no template uses it after unification); fix scaffold_submission() return order in generalization plan doc

Review feedback addressed

Test plan

  • uv run pytest -q — 132 passed
  • No "Made-with: Cursor" in any commit message
  • Verify no template references has_docs
  • Verify scaffold_submission() return order matches doc

Related

Replace Dockerfile.skilled.j2 and Dockerfile.unskilled.j2 with a single
Dockerfile.j2 that uses copy_pairs loop. Update task.toml.j2 to use
skills_dir check instead of variant name. Remove variant-to-template
bridge code from scaffold.py.
Rename skilled-task-dir/unskilled-task-dir results to
treatment-task-dir/control-task-dir. Update step script to use
tasks-treatment/ and tasks-control/ output directories.
Update implementation_plan.md phases 2-6, README "How It Works",
and ab_testing_generalization_plan.md checklists. Add workstreams
roadmap. No code changes — terminology alignment only.
@GuyZivRH
GuyZivRH force-pushed the APPENG-4932/ab-eval-flow-templates branch from 283ba17 to a998b1b Compare April 14, 2026 09:50
Remove has_docs from _build_template_context — no template references
it after Dockerfile unification (docs handled via copy_pairs).
Fix scaffold_submission() return order in generalization plan doc:
(treatment_dir, control_dir), not (control_dir, treatment_dir).
@GuyZivRH
GuyZivRH force-pushed the APPENG-4932/ab-eval-flow-templates branch from a998b1b to 5793f45 Compare April 14, 2026 10:03
Sync code snippets with actual validators (CopySpec._validate_dest,
customize_context submission_dir param, _PerVariantStrategy base).
Check all commit checklists, fix Dockerfile.j2 snippet, correct
copy_pairs descriptions, update branch/PR info and files table.
Stop tracking Docs/ab_testing_generalization_plan.md — it is a
local working document, not a deliverable. The file remains on
disk for reference.
@GuyZivRH
GuyZivRH requested review from dmartinol and r2dedios April 14, 2026 10:29

@r2dedios r2dedios left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Just a small question from my side

Comment thread pipeline/tasks/scaffold.yaml Outdated
@@ -60,13 +60,13 @@ spec:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is pydantic missing here? It is not installed by default in: registry.access.redhat.com/ubi9/python-311:latest

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right! good catch! validate.yaml task already installs pydantic but it was only missing from the scaffold task. the fix I adds pydantic to the pip install line in scaffold.yaml

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scaffold.py imports abevalflow.schemas (Pydantic models) but the
Tekton task only installed jinja2 and pyyaml. pydantic is not
bundled in ubi9/python-311.
@GuyZivRH
GuyZivRH requested a review from r2dedios April 14, 2026 11:52

@r2dedios r2dedios left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GuyZivRH
GuyZivRH merged commit 67cb05c into main Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants