Repository navigation
fix_build_error refactoring - #498
Conversation
Let log files persist in the working directory so the backport fix workflow can preserve them for subsequent fix iterations. Assisted-by: Claude Opus 4.6
Fall back to fork_and_prepare_dist_git (full reset) if the upstream repository is missing when entering the incremental fix loop. Assisted-by: Claude Opus 4.6
Move .log.gz files left by the build agent into the upstream repo's build-logs/attempt-0/ directory so the fix agent can access them. Assisted-by: Claude Opus 4.6
…ations Write a structured log file before each fix agent invocation so subsequent iterations can see what was already tried and avoid repeating failed strategies. Assisted-by: Claude Opus 4.6
Replace the verbose 143-line prompts with a concise 55-line version. Remove git command examples, log-reading micromanagement, and redundant rules. Add references to fix-attempts.md and build-logs. ZSTREAM variant now references the same constant since both were identical. Assisted-by: Claude Opus 4.6
There was a problem hiding this comment.
Code Review
This pull request enhances the backport agent's ability to handle build failures by implementing a structured tracking system for fix attempts. Key changes include refactoring the backport agent's prompt for conciseness, organizing build logs into attempt-specific directories, and maintaining a historical log of fix attempts in a markdown file. Additionally, the build agent no longer deletes log files, and a YAML syntax fix was applied to the compose configuration. A review comment suggests refactoring the logic for writing to the fix-attempts log to reduce code duplication and improve maintainability.
|
@opohorel hmm, how did you push this? :D the pre-push check should have stopped you, because you did not update the skill. |
TomasTomecek
left a comment
There was a problem hiding this comment.
+89-267 love to see such PRs that reduce tech debt
| f.write( | ||
| f"\n## Attempt {attempt_num}\n\n" | ||
| f"**Build error to fix:**\n```\n{state.build_error}\n```\n\n" | ||
| ) |
There was a problem hiding this comment.
would it be a big hassle to cover this new code with a test case?
There was a problem hiding this comment.
hmm, I'll think about it. I would love to get some e2e testing for the backport agent
There was a problem hiding this comment.
I've added tests for the helpers functions
good question, simple answer. I force pushed into my fork as I was rewriting history. I'll regenerate the skills 😅 |
Even force push triggers pre-push hook. Do you have pre-commit properly installed? |
Consolidate duplicated log-moving logic from run_build_agent and fix_build_error into a single private helper, placed alongside the other utility functions after the prompt definitions. Assisted-by: Claude Opus 4.6
Assisted-by: Claude Opus 4.6
you are right, I remember installing it and using it, but somehow it didn't trigger here. I reinstalled it and now it warns me |
Assisted-by: Claude Opus 4.6
Assisted-by: Claude Opus 4.6
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a structured tracking system for incremental build fix attempts. It introduces 'justification' and 'fix_version' parameters to the backport skill, refactors the build error fix prompt for conciseness, and adds logic to move build logs into versioned directories while maintaining a 'fix-attempts.md' log. Review feedback identifies a conflict between agent instructions and orchestrator logic regarding log locations, an off-by-one error in directory naming, and the need to use 'shutil.move' for cross-filesystem reliability. Additionally, improvements were suggested to reduce redundancy in the generated fix attempts log.
Assisted-by: Claude Opus 4.6
Summary
This PR improves the
fix_build_erroriterative workflow for the backport agent by adding persistent memory across fix iterations and slimming down the prompts.Changes
Persistent memory for fix iterations:
builder-live.log,root.log) are now preserved in<upstream-repo>/build-logs/attempt-N/directories across fix attempts, instead of being deleted after analysis.fix-attempts.mdfile is seeded and appended on each iteration with the build error, giving each fresh fix agent context on what was already tried.Prompt optimization:
fix-attempts.mdbefore starting and not repeat failed strategies.Misc:
restart: noYAML parsing bug incompose.yaml(interpreted as booleanFalse).Commits
build_agent: stop deleting build logs after analysisbackport_agent: guard against missing upstream repo in fix_build_errorbackport_agent: preserve build logs for fix iterationsbuild-logs/attempt-0/on initial failurebackport_agent: seed fix-attempts.md as persistent memory across iterationsbackport_agent: slim fix_build_error promptsfix restart error of supervisor containerrestart: no→restart: "no"