Skip to content
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

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from

Conversation

majdyz
Copy link
Contributor

@majdyz majdyz commented Mar 6, 2025

Changes 🏗️

In the case of concurrent output, static links end up being re-used twice without although not producing any output.

Consider this scenario:

A ---- B
       |  
C-----/

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 its value 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 of format pin, the FillTextTemplate should only be executed once.

Correct behavior:
image

Fill text Template is producing:

  • Hi Person-B
  • One incomplete execution waiting for format field to be provided.

Current behavior:
image

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:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • described on the description above.

@majdyz majdyz requested a review from a team as a code owner March 6, 2025 16:53
@majdyz majdyz requested review from ntindle and Bentlybro and removed request for a team March 6, 2025 16:53
@github-actions github-actions bot added platform/backend AutoGPT Platform - Back end platform/blocks labels Mar 6, 2025
Copy link

qodo-merge-pro bot commented Mar 6, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Debug Print

There's a debug print statement that should be removed before merging. The print statement on line 564 is outputting debugging information that shouldn't be in production code.

print(">>>>>>>>> from links", links, "latest_output", latest_output)
String Formatting

The string formatting in the _wrap method has been changed to use an empty string when extra is falsy, but the syntax looks unusual with double quotes inside single quotes.

return f"{self.prefix} {msg} {extra or ""}"

Copy link

netlify bot commented Mar 6, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 8df4625
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67d114a1de87200008ce53b1

@github-actions github-actions bot added the size/m label Mar 6, 2025
Copy link

deepsource-io bot commented Mar 6, 2025

Here's the code health analysis summary for commits f37d4c2..8df4625. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗
DeepSource Python LogoPython✅ Success
❗ 20 occurences introduced
🎯 19 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

netlify bot commented Mar 6, 2025

Deploy Preview for auto-gpt-docs ready!

Name Link
🔨 Latest commit 1215fef
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67c9d31ee624c50008ed32c5
😎 Deploy Preview https://deploy-preview-9593--auto-gpt-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@AutoGPT-Agent
Copy link

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.

Copy link

netlify bot commented Mar 6, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 8df4625
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67d114a10ef3a60008fc02ce

@AutoGPT-Agent
Copy link

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.

@github-actions github-actions bot added size/l and removed size/m labels Mar 7, 2025
@AutoGPT-Agent
Copy link

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.

@majdyz majdyz force-pushed the zamilmajdy/fix-static-output-resolve branch from 4f74af2 to 131d9d2 Compare March 7, 2025 08:52
@github-actions github-actions bot added conflicts Automatically applied to PRs with merge conflicts and removed platform/blocks labels Mar 7, 2025
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Mar 12, 2025
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🆕 Needs initial review
Development

Successfully merging this pull request may close these issues.

2 participants