Skip to content

fix: return fetched content to LLM in web_fetch tool#833

Merged
xiaket merged 2 commits intosipeed:mainfrom
lucamartinetti:fix/web-fetch-forlm-content
Mar 1, 2026
Merged

fix: return fetched content to LLM in web_fetch tool#833
xiaket merged 2 commits intosipeed:mainfrom
lucamartinetti:fix/web-fetch-forlm-content

Conversation

@lucamartinetti
Copy link
Contributor

Summary

  • WebFetchTool.Execute was setting ForLLM to a summary string ("Fetched N bytes from URL (extractor: ..., truncated: ...)") instead of the actual extracted text
  • This was introduced in ca781d4 when the Tool interface was refactored from (string, error) to *ToolResult — the old code returned the full content as a single string, but the split into ForLLM/ForUser accidentally hid the content from the LLM
  • The model never saw the fetched page content and could not answer questions based on web fetches

Fix

Return the extracted text in ForLLM (the field the LLM actually reads) instead of the metadata summary. ForUser continues to return the full JSON result.

Test plan

  • Deployed in a sandboxed container and verified web_fetch now returns page content the LLM can use (tested with weather data queries)

🤖 Generated with Claude Code

WebFetchTool.Execute was setting ForLLM to a summary string
("Fetched N bytes from URL ...") instead of the actual extracted
text. This meant the LLM never saw the page content and could not
answer questions based on fetched web pages.

Return the extracted text in ForLLM so the model can use it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 26, 2026 19:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes web_fetch so the LLM receives the actual extracted page content (instead of a metadata-only summary), restoring the tool’s usefulness for answering questions based on fetched URLs.

Changes:

  • Update WebFetchTool.Execute to return extracted text in ToolResult.ForLLM.
  • Keep ToolResult.ForUser as the full JSON payload (url/status/extractor/truncated/length/text).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/tools/web.go Outdated
Comment on lines 654 to 656
return &ToolResult{
ForLLM: fmt.Sprintf(
"Fetched %d bytes from %s (extractor: %s, truncated: %v)",
len(text),
urlStr,
extractor,
truncated,
),
ForLLM: text,
ForUser: string(resultJSON),
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

This change will break existing unit tests that assert WebFetchTool.ForLLM contains a summary string (see pkg/tools/web_test.go:40-43). Update the test expectations (and any other callers) to validate that ForLLM now contains the extracted text content instead of metadata, or include the needed metadata in ForLLM if the summary is still required by contract.

Copilot uses AI. Check for mistakes.
@afjcjsbx
Copy link
Collaborator

great bug you found! but instead of replacing ForLLM with text I would do something like this:

	return &ToolResult{
		ForLLM: string(resultJSON),
		ForUser: fmt.Sprintf(
			"Fetched %d bytes from %s (extractor: %s, truncated: %v)",
			len(text),
			urlStr,
			extractor,
			truncated,
		),
	}

there are also unit tests to fix, but nice PR!

Accept suggestion from afjcjsbx: the LLM should receive the full JSON
result (including extracted text) while the user sees a short summary.
Update tests to match the new field assignment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lucamartinetti
Copy link
Contributor Author

Thanks @afjcjsbx, good call! Applied your suggestion — ForLLM now gets the full JSON result and ForUser gets the summary. Also updated the tests to match.

@afjcjsbx
Copy link
Collaborator

LGTM!

Copy link
Collaborator

@PixelTux PixelTux left a comment

Choose a reason for hiding this comment

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

LGTM

@afjcjsbx
Copy link
Collaborator

@lxowalle PTAL

@afjcjsbx afjcjsbx mentioned this pull request Feb 28, 2026
10 tasks
@afjcjsbx
Copy link
Collaborator

afjcjsbx commented Mar 1, 2026

@xiaket PTAL

Copy link
Collaborator

@xiaket xiaket left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

@xiaket xiaket merged commit fc9f1ec into sipeed:main Mar 1, 2026
2 checks passed
@Orgmar
Copy link
Contributor

Orgmar commented Mar 2, 2026

@lucamartinetti Good catch on the web_fetch bug. Returning the summary metadata string instead of the actual fetched content to the LLM would make the tool basically useless for real web lookups. Subtle regression from the ToolResult refactor.

We have a PicoClaw Dev Group on Discord where contributors chat and collaborate. If you're interested, send an email to support@sipeed.com with the subject [Join PicoClaw Dev Group] lucamartinetti and we'll send you the invite link.

liangzhang-keepmoving pushed a commit to liangzhang-keepmoving/picoclaw that referenced this pull request Mar 2, 2026
* fix: return fetched content to LLM in web_fetch tool

WebFetchTool.Execute was setting ForLLM to a summary string
("Fetched N bytes from URL ...") instead of the actual extracted
text. This meant the LLM never saw the page content and could not
answer questions based on fetched web pages.

Return the extracted text in ForLLM so the model can use it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: put full JSON result in ForLLM, summary in ForUser

Accept suggestion from afjcjsbx: the LLM should receive the full JSON
result (including extracted text) while the user sees a short summary.
Update tests to match the new field assignment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
hyperwd pushed a commit to hyperwd/picoclaw that referenced this pull request Mar 5, 2026
* fix: return fetched content to LLM in web_fetch tool

WebFetchTool.Execute was setting ForLLM to a summary string
("Fetched N bytes from URL ...") instead of the actual extracted
text. This meant the LLM never saw the page content and could not
answer questions based on fetched web pages.

Return the extracted text in ForLLM so the model can use it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: put full JSON result in ForLLM, summary in ForUser

Accept suggestion from afjcjsbx: the LLM should receive the full JSON
result (including extracted text) while the user sees a short summary.
Update tests to match the new field assignment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Pluckypan pushed a commit to Pluckypan/picoclaw that referenced this pull request Mar 6, 2026
* fix: return fetched content to LLM in web_fetch tool

WebFetchTool.Execute was setting ForLLM to a summary string
("Fetched N bytes from URL ...") instead of the actual extracted
text. This meant the LLM never saw the page content and could not
answer questions based on fetched web pages.

Return the extracted text in ForLLM so the model can use it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: put full JSON result in ForLLM, summary in ForUser

Accept suggestion from afjcjsbx: the LLM should receive the full JSON
result (including extracted text) while the user sees a short summary.
Update tests to match the new field assignment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants