Skip to content

Conversation

@abhisheksp
Copy link

@abhisheksp abhisheksp commented Dec 28, 2025

Description

This PR adds append mode support to the file_write tool, resolving the discrepancy between documentation and implementation where append functionality was mentioned but not implemented.

Changes:

  • Added optional mode parameter accepting "write" (default) or "append"
  • Maintains backward compatibility - default behavior unchanged (overwrites file)
  • Added validation for invalid mode values
  • Enhanced UI panel to display the selected write mode
  • Updated module and function documentation with usage examples
  • Added botocore[crt] to dev dependencies to support agent interface tests

Related Issues

Fixes #344

Documentation PR

N/A - Documentation updated in this PR (module docstrings and function docstrings)

Type of Change

Enhancement (new feature that adds functionality)

Testing

Test Coverage:

  • All tests pass: 16/16 for file_write module ✅
  • Added 7 new test cases covering append mode functionality
  • All existing tests continue to pass (100% backward compatible)

Pre-commit:

  • I ran `hatch run prepare` - passes with Python 3.13

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

- Add optional 'mode' parameter with 'write' (default) and 'append' options
- Maintain backward compatibility with default overwrite behavior

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@abhisheksp abhisheksp force-pushed the feat/file-write-append-mode branch from 46ccd01 to 24d996b Compare December 28, 2025 20:29
"pytest>=8.0.0,<9.0.0",
"ruff>=0.13.0,<0.14.0",
"responses>=0.6.1,<1.0.0",
"botocore[crt]>=1.39.7,<2.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Hi, why do we need a dependency change as part of this PR?

Copy link
Member

Choose a reason for hiding this comment

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

Which test would fail regarding Added botocore[crt] to dev dependencies to support agent interface tests

Copy link
Author

Choose a reason for hiding this comment

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

The botocore[crt] dependency fixes a pre-existing test failure in the agent interface tests.

The failure:

  • test_file_write_via_agent
  • test_file_write_append_via_agent

Both error with:

botocore.exceptions.MissingDependencyException: Using the login credential 
provider requires an additional dependency. You will need to pip install 
"botocore[crt]" before proceeding.

This isn't related to append mode - it's been broken. I caught it while running the full test suite. With botocore[crt] added, all 16 tests pass.

If you'd prefer, I can split this into a separate PR and keep this one focused on just the append mode feature. The core functionality (14 tests) works fine without it.

@cagataycali
Copy link
Member

🤖 Review by strands-coder autonomous agent

Great work on this PR! I reviewed the implementation and it looks solid:

✅ What I Liked

  • Clean implementation with backward compatibility preserved
  • Comprehensive test coverage (7 new tests)
  • Good validation for invalid mode values
  • UI panel update to show write mode

💡 Suggestion for the dependency discussion

I agree with splitting concerns - the botocore[crt] fix could be a separate PR to keep this one focused on the append mode feature. The core append functionality (14 tests) works independently.

CI Note

The check-access-and-checkout failure is expected for fork PRs - maintainer workflow approval is needed. All actual tests pass! ✅

Nice contribution @abhisheksp! 🦆


Review by strands-coder autonomous agent

@strands-agent
Copy link
Contributor

🎉 Excellent Work on Append Mode Implementation!

@abhisheksp - This is a high-quality contribution that perfectly addresses issue #344. Your implementation is clean, well-tested, and maintains backward compatibility.

✅ What's Great

  1. Perfect Issue Resolution

  2. Clean Implementation 🎯

    • Simple, elegant mode parameter with "write" (default) and "append" options
    • Proper validation with clear error messages for invalid modes
    • Maps cleanly to Python file modes ("w" and "a")
  3. Excellent Test Coverage 🧪

    • 7 comprehensive test cases covering all scenarios
    • Tests append to existing files
    • Tests append creating new files
    • Tests default mode behavior
    • Tests explicit write mode
    • Tests invalid mode validation
    • Tests multiple consecutive appends
    • All tests passing (16/16) ✅
  4. Documentation 📚

    • Updated module docstring with clear examples
    • Enhanced function docstring
    • Added usage examples for both write and append modes

🔍 CI Status Analysis

I see the "check-access-and-checkout" failure - this is NOT a problem with your code. This is a common CI configuration issue for external contributors where the workflow needs maintainer approval to run on forks.

All actual tests are passing:

  • ✅ Unit Tests: Python 3.10-3.13 (Linux, Windows, macOS) - ALL SUCCESS
  • ✅ Dependency Checks: All versions - ALL SUCCESS
  • ✅ Lint: SUCCESS

The check-access-and-checkout failure is just a workflow permission check unrelated to code quality.

💡 Code Review

Your implementation is solid. The code follows best practices:

# Clean validation
if mode not in ["write", "append"]:
    error_message = f"Invalid mode: '{mode}'. Must be 'write' or 'append'"
    # ... proper error handling

# Simple mapping to file modes
file_mode = "w" if mode == "write" else "a"

The UI panel update to show the mode is a nice touch:

("\nMode: ", "cyan"),
(mode, "yellow"),

🚀 Ready to Merge

This PR is production-ready and should be merged:

  • ✅ Solves the documented issue completely
  • ✅ Backward compatible (default behavior unchanged)
  • ✅ Comprehensive test coverage
  • ✅ All real tests passing
  • ✅ Well documented
  • ✅ Follows project conventions
  • ✅ Clean, maintainable code

🏆 Great First Contribution

This is exactly the kind of contribution that strengthens the project. You:

  1. Identified a real documentation/implementation mismatch
  2. Opened a clear issue
  3. Implemented a clean solution
  4. Added thorough tests
  5. Updated documentation

Thank you for this excellent contribution to strands-agents/tools! 🙌


Review by strands-coder autonomous agent 🤖 | Run: #20939569621

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.

file_write tool missing append support despite documentation claims

4 participants