Skip to content

ADFA-6836 | Let create_file take empty content - #156

Merged
davidschachterADFA merged 3 commits into
mainfrom
ADFA-6836
Nov 3, 2026
Merged

davidschachterADFA merged 3 commits into
mainfrom
ADFA-6836

Conversation

@davidschachterADFA

Copy link
Copy Markdown
Contributor

Summary

ADFA-6836. create_file with content="" (an empty file) was rejected by the executor's required-argument check as "Missing required argument(s): content", before the approval dialog. A small on-device model asked to "add a file Util.kt with an empty class Util" sends exactly that, then answers "I can't assist with that request" and stops.

For create_file, content now counts as missing only when it is absent. An empty string goes through to the approval dialog; an omitted content is still rejected. update_file, edit_file and every other tool keep the old check. The change is an EMPTY_ALLOWED_ARGS set in plugins/AI-Core/.../tool/Executor.kt, keyed on the registered tool name and argument, plus two tests in ExecutorTest.

Measured on a Pixel 6 Pro (Android 17)

qwen2.5-coder-1.5b-instruct-q4_k_m, "Build Apps Anywhere" test project, a fresh chat, prompt "Add a file Util.kt with an empty class Util", approval declined.

Build Result, 3 of 3 runs
Before (on #155's build) create_file(…/Util.kt, content="") → "Missing required argument(s): content"; next turn "I can't assist with that request."; no dialog
This change create_file(…/Util.kt, content="") reaches the approval dialog

Review

An independent read-only review found no blocking problems. Empty content passes every later step it checked (alias normalisation, CreateFileHandler.execute, the path guard, the approval text), an omitted content is still rejected, and only the new empty-content test fails without the change. It noted one existing hazard, not caused by this diff: a text-parsed call with JSON "content": null becomes the string "null", which the old check also accepted, so such a call would write a file containing null. Not changed here.

Not verified

  • Approving the dialog and seeing the empty file created: the harness declines every write, so that step rests on the existing handler (execute writes "").
  • The second half of ADFA-6836 (the model answering "I can't assist" after a tool error) was not tested separately; it did not occur here because there is no error to react to.
  • AI-Core was built from main plus this change, with the AI-Agent-Local build from ADFA-6805 | Teach the local model to read files with read_file, not search #155 installed; this change does not touch AI-Agent-Local.
  • One phone, one model file, three runs.
  • One AI-Core run had a single failure in ChatViewModelCompactionTest, which this change does not touch; it passed on the next two full runs. I treat it as a flake and have not filed it.

Test plan

  • AI-Core testDebugUnitTest (1,257 tests; passes on a rerun after the one flake above)
  • Same-phone check above, three runs
  • Independent review
  • addons check exits 0

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vh2DAaJxvRXKRYkgiDqwGQ

The required-argument check read content="" as missing, so "add a file
with an empty class" failed with "Missing required argument(s): content"
before reaching the approval dialog. For create_file, content now counts
as missing only when it is absent. An omitted content is still rejected.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vh2DAaJxvRXKRYkgiDqwGQ
@davidschachterADFA

Copy link
Copy Markdown
Contributor Author

Independent review (code-review skill, high) at df79d3be

An independent reviewer read the diff and returned 5 findings. I then checked each against the code. None blocks the merge and I made no change.

ID Finding Checked Conclusion
F06 update_file still rejects content = "", though UpdateFileHandler would write it True: only create_file/content is in EMPTY_ALLOWED_ARGS. If an empty update_file reached the handler it would truncate an existing file to zero length, after the approval step, and report "Updated file: X (0 characters)" Left as it is: this PR is about create_file, and blanking a file is riskier than creating one. Whether to allow it is a decision for the owner
F07 Three places now say when an empty string is legal (EMPTY_ALLOWED_ARGS, VERBATIM_TEXT_ARGS, a comment in requiredArgsForTool) True A design point, not a defect
F08 The schema still marks content required while the handler has ?: "" The two agree: the key must be present and "" is a valid value. The handler fallback is unreachable through the Executor Not a defect. Nothing tells the model that empty is allowed
F09 Tests cover only "" and an absent key True. Whitespace-only content is now accepted and untested. A JSON null arrives as JSONObject.NULL, whose toString() is "null", so it passes and a file would contain null The null behaviour is not new. Tests could be added
F10 A non-string content (a number, boolean or object) is stringified and passes the check True Not new: the old check let these through too

Not device-verified.

Posted by Claude Code for David Schachter.

@davidschachterADFA
davidschachterADFA merged commit 164d8cb into main Nov 3, 2026
1 check passed
@davidschachterADFA
davidschachterADFA deleted the ADFA-6836 branch November 3, 2026 14:42
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.

3 participants