-
Notifications
You must be signed in to change notification settings - Fork 45.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(backend): Fix static link resolving behavior on concurrent output #9593
base: dev
Are you sure you want to change the base?
Conversation
… Anthropics support
… Anthropics support
…opic' into zamilmajdy/improve-sdm-add-anthropic
… Anthropics support
…opic' into zamilmajdy/improve-sdm-add-anthropic
…milmajdy/fix-static-output-resolve
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
✅ Deploy Preview for auto-gpt-docs-dev canceled.
|
Here's the code health analysis summary for commits Analysis Summary
|
✅ Deploy Preview for auto-gpt-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
While the PR title follows convention and the changes appear focused on fixing a specific issue with static link resolving, the PR template is incomplete. Key issues: 1) Missing test plan details despite code changes 2) Checklist items are not checked/completed. However, the changes appear targeted and within scope, touching only backend files. |
✅ Deploy Preview for auto-gpt-docs canceled.
|
The PR has several issues that need to be addressed: 1) The PR template checklist is completely empty and should at least have the code changes section filled out given this is a code change PR. 2) While the title follows conventional commit format with fix(backend), the description of changes could be clearer in explaining what exactly was changed in each file. However, the core fix itself seems valid and the author explains the problem scenario well. The changes appear focused on fixing the static link resolution issue without major out-of-scope modifications. |
The PR has several issues: 1) The title follows conventional commit format but the PR template is not completely filled out - the test plan and checklist sections are empty. 2) However, the changes section is very well documented with clear examples and diagrams showing the current vs desired behavior. 3) The code changes appear focused on fixing a specific issue with static link behavior and concurrent outputs. The implementation looks contained to this scope. 4) There are appropriate data.py changes that handle user_id correctly. |
autogpt_platform/backend/backend/blocks/smart_decision_maker.py
Outdated
Show resolved
Hide resolved
4f74af2
to
131d9d2
Compare
…milmajdy/fix-static-output-resolve
…milmajdy/fix-static-output-resolve
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
…milmajdy/fix-static-output-resolve
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
Changes 🏗️
In the case of concurrent output, static links end up being re-used twice without although not producing any output.
Consider this scenario:
If A-B is static, and an output is produced from C-B, B will try to re-use the output from C because it thinks a static output is connected to it.
Real example:
A
FillTextTemplate
block has itsvalue
pin connected to two different block outputs.The
format
pin is connected to another 2 blocks, but one of them is a static output (StoreValueBlock) that ends up never being executed.wIth two inputs or
value
pin and only one input offormat
pin, theFillTextTemplate
should only be executed once.Correct behavior:

Fill text Template is producing:
Hi Person-B
format
field to be provided.Current behavior:

Fill text Template is producing:
Hi Person-B
Hi Person-A
(The combine text block output is unexpectedly being re-used twice).Checklist 📋
For code changes: