Skip to content

feat(learning): add daily engineer-bot retrospective flow + bump engine to 654a31d0 - #499

Merged
eric-wang-1990 merged 7 commits into
mainfrom
ai/add-learning-retrospective
Aug 13, 2026
Merged

feat(learning): add daily engineer-bot retrospective flow + bump engine to 654a31d0#499
eric-wang-1990 merged 7 commits into
mainfrom
ai/add-learning-retrospective

Conversation

@eric-wang-1990

@eric-wang-1990 eric-wang-1990 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What

Turn on the engineer-bot daily learning (retrospective) loop for this repo, mirroring databricks-sql-python (the reference implementation). Three changes:

File Change
.github/workflows/engineer-bot-learning.yml (new) Daily schedule (17:23 UTC) + workflow_dispatch. Own job on ./.github/actions/bot-prelude (Python + pinned engine install, PAT-free).
.bot/config.yaml Add the retrospective: block + close the loop with author.knowledge_log.
.github/actions/bot-prelude/action.yml Bump engine-ref d05dcb11654a31d0.

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

  • No setup-jfrog step — this repo's 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. Mirrors the read-only reviewer-bot.yml.
  • actions: read added for Track B (lists engineer-bot author runs + downloads their logs via the App token). The engineer-bot App installation must also carry actions:read, or the run 403s.
  • Close the loop: author.knowledge_log == retrospective.log_path (.claude/knowledge/learning-log.md). — the retrospective writes it, the author reads it back. system_prompt is omitted (this repo ships no prompts/retrospective_system.md, so the engine's base prompt is used); the learning log is created on first write.
  • Pin bump is SHA-only — SDK/CLI (0.2.102 / 2.1.61) unchanged at 654a31d0. Brings the daily-cron retrospective + per-bot models.
  • The new workflow is prettier-formatted to match the sibling bot workflows (repo CI runs prettier --check).

Validation

The schedule registers only once this lands on the default branch. Before the first cron, validate via a manual workflow_dispatch with a recent since — it should open/update the ai/learning-pr-* PR.

This pull request and its description were written by Isaac.

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>
Copilot AI lite review requested due to automatic review settings August 13, 2026 02:27

Copilot AI 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.

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-learning workflow triggered on a daily cron and workflow_dispatch.
  • Extend .bot/config.yaml with a retrospective block and connect it to the author phase via author.knowledge_log.
  • Update the shared bot prelude’s pinned engine-ref SHA.

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.

Comment thread .bot/config.yaml Outdated
Comment on lines +123 to +124
system_prompt: prompts/retrospective_system.md
log_path: .claude/knowledge/learning-log.md # MUST match author.knowledge_log above

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +110 to +112
RUNNER_TEMP: ${{ runner.temp }}
SINCE: ${{ inputs.since }}
WINDOW_HOURS: ${{ inputs.window-hours }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 peco-review-bot Bot 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.

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.

Comment thread .bot/config.yaml Outdated
Comment thread .bot/config.yaml
@eric-wang-1990 eric-wang-1990 added the engineer-bot Maintainer-only: triggers the autonomous engineer-bot (bug-fix author on issues; follow-up on PRs) label Aug 13, 2026
…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>

@peco-review-bot peco-review-bot Bot 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.

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.

Comment thread .github/workflows/engineer-bot-learning.yml
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>

@peco-review-bot peco-review-bot Bot 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.

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.

Comment thread .bot/config.yaml
…t form

The learning workflow set MODEL_ENDPOINT to
`.../serving-endpoints/anthropic/invocations`. That is NOT translated the way the
old comment claimed: sdk_agent.translate_endpoint has an early-return for URLs
already containing `/serving-endpoints/anthropic`, which keeps the trailing
`/invocations` — so the CLI appends `/v1/messages` and calls
`.../serving-endpoints/anthropic/invocations/v1/messages`, which the gateway
rejects with `400 Unsupported native API path`.

Proof: databricks-sql-python's learning cron (same endpoint form) has failed
every scheduled run with exactly this 400. The reviewer-bot flagged it on the
sibling PRs (Low), and it is the real cause.

Use the concrete `.../serving-endpoints/databricks-claude-opus-4-8/invocations`
form that reviewer-bot.yml / engineer-bot.yml already use successfully:
translate_endpoint strips `<model>/invocations` to the `.../serving-endpoints/
anthropic` base the CLI needs. Comment corrected to explain the trap.

Co-authored-by: Isaac
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
The model key was added earlier but is RETIRED in the engine (retrospective.py
warns "retrospective.model is no longer honored") — the learning run always uses
the engine-owned engineer-bot model. Keeping it only emits a per-run ::warning::.
Remove it to match the other driver repos.

Co-authored-by: Isaac
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

@peco-review-bot peco-review-bot Bot 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.

✅ No issues identified by the review bot.

@eric-wang-1990
eric-wang-1990 added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 05974d5 Aug 13, 2026
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engineer-bot Maintainer-only: triggers the autonomous engineer-bot (bug-fix author on issues; follow-up on PRs)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants