feat(learning): add daily engineer-bot retrospective flow + bump engine to 654a31d0 - #499
feat(learning): add daily engineer-bot retrospective flow + bump engine to 654a31d0#499eric-wang-1990 wants to merge 5 commits into
Conversation
Turn on the engineer-bot learning loop for this repo, mirroring databricks-sql-python (the reference). Three parts: - engineer-bot-learning.yml (new): daily schedule (17:23 UTC) + workflow_dispatch (string since / window-hours recovery inputs). Own job sharing ./.github/actions/ bot-prelude for Python + pinned engine install, PAT-free. No setup-jfrog step: install-bot-engine self-mints its pip credential via OIDC and never reads PIP_INDEX_URL from env, so a setup-jfrog step would be redundant (the engine install is the only fetch; the learning flow runs no npm build) and would leak a token-bearing PIP_INDEX_URL to the job env. actions:read added for Track B (lists engineer-bot author runs + downloads their logs via the App token). - .bot/config.yaml: add the retrospective block (log_path, branch_prefix, pr_label, plus the repo's existing prompts/retrospective_system.md as additive guidance) and close the loop with author.knowledge_log matching retrospective.log_path (.claude/knowledge/learning-log.md), so the author phase reads back what the retrospective learns. - bot-prelude: bump engine-ref d05dcb11 to 654a31d0 (engine main) so the pinned engine has the daily-cron retrospective + per-bot models. SDK/CLI (0.2.102 / 2.1.61) unchanged at that SHA, so this is a SHA-only bump. The schedule registers only once this lands on the default branch; validate before the first cron via a manual workflow_dispatch with a recent since. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Pull request overview
Enables the engineer-bot daily “learning/retrospective” loop for this repository by adding a scheduled+manual GitHub Actions workflow, wiring the retrospective configuration into .bot/config.yaml, and bumping the pinned bot-engine SHA via the shared bot-prelude composite action.
Changes:
- Add a new
engineer-bot-learningworkflow triggered on a daily cron andworkflow_dispatch. - Extend
.bot/config.yamlwith aretrospectiveblock and connect it to the author phase viaauthor.knowledge_log. - Update the shared bot prelude’s pinned
engine-refSHA.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/engineer-bot-learning.yml |
Adds scheduled/manual workflow that runs the retrospective engine entrypoint and pushes/updates the rolling learning PR. |
.bot/config.yaml |
Adds retrospective configuration and connects retrospective output log back into the author flow. |
.github/actions/bot-prelude/action.yml |
Bumps the pinned engine commit SHA used across bot workflows. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| system_prompt: prompts/retrospective_system.md | ||
| log_path: .claude/knowledge/learning-log.md # MUST match author.knowledge_log above |
There was a problem hiding this comment.
Applied the fix locally but the push failed (likely another commit landed on this branch since I started). This bundle covered 2 thread(s); all reverted. Re-trigger by replying to this thread.
[rebase failed]
error: cannot pull with rebase: You have unstaged changes.
error: Please commit or stash them.
| RUNNER_TEMP: ${{ runner.temp }} | ||
| SINCE: ${{ inputs.since }} | ||
| WINDOW_HOURS: ${{ inputs.window-hours }} |
There was a problem hiding this comment.
⛔ NEEDS HUMAN DECISION — the bots can't resolve this thread; a maintainer's input is required.
The reviewer's claim is valid: inside ${{ }}, the hyphen in inputs.window-hours is parsed as subtraction (inputs.window - hours), yielding an empty value rather than the workflow_dispatch input. The fix is to use bracket notation ${{ inputs['window-hours'] }} (and the same for the SINCE/window-hours usages) or rename the input to window_hours. However, this comment is on .github/workflows/engineer-bot-learning.yml, and .github/ is a denied/non-writable path for this job's tools (read_file and edit_file both return "Path denied or invalid"). I cannot make or verify the change here, so a human must apply the bracket-notation fix in a change that is permitted to touch .github/.
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 High · 1 Medium
Wiring for the daily retrospective flow (new workflow, prelude engine-pin bump, config block) is otherwise sound and faithfully mirrors the sibling reviewer-bot.yml — but two files the config points at, and which the PR description explicitly claims are already present/seeded, do not exist in the checkout: prompts/retrospective_system.md (High) and .claude/knowledge/learning-log.md (Medium). Verify engine tolerance for the missing paths or add the files before enabling the cron.
eric-wang-1990
added
the
engineer-bot
…is repo) The retrospective block pointed system_prompt at prompts/retrospective_system.md, which does not exist in this repo (only databricks-sql-kernel ships one; the sql-python reference correctly omits the key). The engine treats a set-but-missing system_prompt as a HARD ERROR, so the daily cron would fail every run. Drop the key so the flow uses the engine's built-in base prompt (engineer_prompts.RETRO_SYSTEM_PROMPT) — matching databricks-sql-python and the odbc learning PR. Also corrects the config comment, which wrongly claimed the file was "present in this repo." Caught by peco-review-bot on the sibling PRs. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a clean, well-documented infra/config PR that faithfully mirrors the sibling reviewer-bot.yml/engineer-bot.yml patterns (local prelude, persist-credentials:false + authenticated push remote, no setup-jfrog, OIDC + actions: read, single-sourced engine pin). One low note: the learning flow doesn't pin a model (retrospective.model unset) while the other bots do — worth confirming the engine default is intended.
Addresses: - #3771964358 at .bot/config.yaml:103 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Addresses: - #3772141334 at .github/workflows/engineer-bot-learning.yml:101 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Addresses: - #3771964358 at .bot/config.yaml:103 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean CI onboarding that mirrors the sibling reviewer/engineer bots (OIDC, azure-prod environment, no setup-jfrog, single-sourced engine pin, authenticated push remote). One low: confirm branch_prefix: ai/learning-pr- (trailing dash + * in the validation note) actually resolves to the single stable rolling branch the comments describe, rather than a per-run suffixed branch.
| # place the effective retrospective model is chosen.) | ||
| model: databricks-claude-opus-4-8 | ||
| log_path: .claude/knowledge/learning-log.md # MUST match author.knowledge_log above | ||
| branch_prefix: ai/learning-pr- |
There was a problem hiding this comment.
🔵 Low — The stated "one rolling PR on a stable branch" design and the configured branch_prefix may be in tension. The workflow header comment and PR body describe a single rolling PR on the stable branch ai/learning-pr that appends a dated section daily until a human merges it. But branch_prefix: ai/learning-pr- has a trailing dash (implying the engine appends a per-run suffix), and the PR's own validation note refers to the target as ai/learning-pr-*. If the engine appends a suffix (e.g. a date/window) to branch_prefix, each run would cut a new branch/PR rather than updating one rolling PR — contradicting the "append daily until merged" human-gated design. Please confirm against the engine's retrospective branch-naming behavior that branch_prefix: ai/learning-pr- actually resolves to a single stable branch (and reconcile the ai/learning-pr vs ai/learning-pr-* wording). If the engine treats this key as an exact branch name rather than a prefix, the trailing dash would also bake a stray dash into the branch name.
There was a problem hiding this comment.
⛔ NEEDS HUMAN DECISION — the bots can't resolve this thread; a maintainer's input is required.
Valid concern, but not resolvable in this repo. The deciding fact — how the retrospective flow derives its branch from branch_prefix (stable branch vs. per-run suffix like bug-fix appends the issue number) — lives in the databricks_bot_engine Python package, which is not vendored here and is outside the writable tree, so I cannot verify it. A human must confirm against the engine's retrospective branch-naming behavior: (1) whether branch_prefix: ai/learning-pr- yields one stable rolling branch or cuts a new branch/PR per run, (2) if stable, whether the trailing dash bakes a stray - into the branch name, and (3) reconcile the ai/learning-pr vs ai/learning-pr-* wording in the header/PR body accordingly. Any config edit here would be a guess at engine semantics I can't inspect.
What
Turn on the engineer-bot daily learning (retrospective) loop for this repo, mirroring
databricks-sql-python(the reference implementation). Three changes:.github/workflows/engineer-bot-learning.yml(new)schedule(17:23 UTC) +workflow_dispatch. Own job on./.github/actions/bot-prelude(Python + pinned engine install, PAT-free)..bot/config.yamlretrospective:block + close the loop withauthor.knowledge_log..github/actions/bot-prelude/action.ymlengine-refd05dcb11→654a31d0.Why
The other bots (reviewer, engineer, followups) were onboarded earlier; the retrospective was deliberately deferred. This adds it. Over an adaptive look-back window the engine gathers merged PRs + engineer-bot author-run logs itself and, if it finds durable learnings, opens one rolling PR (
ai/learning-pr) appending a dated section to.claude/knowledge/learning-log.md— human-gated, never committing the canonical log directly.Notes on the wiring
setup-jfrogstep — this repo'sinstall-bot-engineself-mints its pip credential via OIDC and never readsPIP_INDEX_URLfrom env, so a setup-jfrog step would be redundant (the engine install is the only fetch — the learning flow runs nonpmbuild) and would leak a token-bearingPIP_INDEX_URLto the job env. Mirrors the read-only reviewer-bot.yml.actions: readadded for Track B (lists engineer-bot author runs + downloads their logs via the App token). The engineer-bot App installation must also carryactions:read, or the run 403s.author.knowledge_log==retrospective.log_path(.claude/knowledge/learning-log.md). — the retrospective writes it, the author reads it back.system_promptis omitted (this repo ships noprompts/retrospective_system.md, so the engine's base prompt is used); the learning log is created on first write.0.2.102/2.1.61) unchanged at654a31d0. Brings the daily-cron retrospective + per-bot models.prettier --check).Validation
The
scheduleregisters only once this lands on the default branch. Before the first cron, validate via a manual workflow_dispatch with a recentsince— it should open/update theai/learning-pr-*PR.This pull request and its description were written by Isaac.