Aider fails to make change after showing correct diffs

[aelaguiz]

Issue

After a chat-mode ask I told aider to make the change, it showed diffs but didn't actually make the change even though diffs were correct.

Tokens: 21,317 sent, 251 received. Cost: $0.07 request, $1.39 session.
Committing src/algorithms/card_algorithms.h before applying edits.
Commit c12345d feat: Add support for reserved cards in card algorithm functions
Applied edit to src/algorithms/card_algorithms.h
Commit 482f1cb feat: Add optional reserved cards parameter to enumerate all & enumerate street range
You can use /undo to revert and discard commit 482f1cb.
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
src/algorithms/card_algorithms.cpp src/algorithms/card_algorithms.h src/config/config.h src/config/game_config.h src/tests/test_card_algorithms.cpp src/tests/test_wml_rollout_calculator.cpp
> /ask what is causing this CMakeFiles/test_wml_rollout_calculator.dir/src/tests/test_wml_rollout_calculator.cpp.o -c /Users/aelaguiz/workspace/amirbot/csrc/amirbot/src/tests/test_wml_rollout_calculator.c
pp
/Users/aelaguiz/workspace/amirbot/csrc/amirbot/src/tests/test_wml_rollout_calculator.cpp:124:45: error: call to implicitly-deleted copy constructor of 'std::unique_ptr<CardAlgorithms>'
                        card_algorithms->enumerate_street_range([=](const std::vector<Card>& opponent_hole_cards) {
                                                                 ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__memory/unique_ptr.h:213:59: note: copy constructor is implicitly deleted because 'un
ique_ptr<amirbot::CardAlgorithms>' has a user-declared move constructor
  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
                                                          ^
/Users/aelaguiz/workspace/amirbot/csrc/amirbot/src/tests/test_wml_rollout_calculator.cpp:118:66: error: call to implicitly-deleted copy constructor of 'std::unique_ptr<CardAlgorithms>'
                card_algorithms->enumerate_hands(Street::Type(target_street), [=](const std::vector<Card>& hole_cards, const std::vector<Card>& flop) {
                                                                               ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__memory/unique_ptr.h:213:59: note: copy constructor is implicitly deleted because 'un
ique_ptr<amirbot::CardAlgorithms>' has a user-declared move constructor
  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
                                                          ^
In file included from /Users/aelaguiz/workspace/amirbot/csrc/amirbot/src/tests/test_wml_rollout_calculator.cpp:1:
In file included from /Users/aelaguiz/workspace/amirbot/csrc/amirbot/build/vcpkg_installed/arm64-osx/include/gtest/gtest.h:54:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/iomanip:47:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/istream:170:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/ostream:170:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__memory/shared_ptr.h:27:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1

The error is occurring because you're trying to capture a std::unique_ptr by value in a lambda function. std::unique_ptr cannot be copied, only moved.

To fix this, you have a few options:

 1 Capture by reference instead of by value:

 card_algorithms->enumerate_hands(Street::Type(target_street), [&](const std::vector<Card>& hole_cards, const std::vector<Card>& flop) {
     // ...
 });

 2 Use a raw pointer or reference in the capture:

 CardAlgorithms* card_algorithms_ptr = card_algorithms.get();
 card_algorithms_ptr->enumerate_hands(Street::Type(target_street), [=](const std::vector<Card>& hole_cards, const std::vector<Card>& flop) {
     // ...
 });

 3 If you need to capture other variables by value, you can use a mixed capture:

 card_algorithms->enumerate_hands(Street::Type(target_street), [&card_algorithms, =](const std::vector<Card>& hole_cards, const std::vector<Card>& flop) {
     // ...
 });

The same applies to the inner lambda where you're calling enumerate_street_range.

Choose the option that best fits your needs and the lifetime requirements of the objects you're capturing. Option 1 (capturing by reference) is usually the simplest if you're sure that card_algorithms
will remain valid for the entire lifetime of the lambda.

Tokens: 22,055 sent, 305 received. Cost: $0.07 request, $0.07 session.
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
src/algorithms/card_algorithms.cpp src/algorithms/card_algorithms.h src/config/config.h src/config/game_config.h src/tests/test_card_algorithms.cpp src/tests/test_wml_rollout_calculator.cpp
> /ask what about just making ti a shared pointer

Yes, using a std::shared_ptr instead of a std::unique_ptr is another valid solution. This would allow you to copy the pointer in the lambda capture. Here's how you can modify your code:

 1 First, change the declaration of card_algorithms in your HandRolloutTest class:

 std::shared_ptr<CardAlgorithms> card_algorithms;

 2 In the SetUp() function, initialize it like this:

 card_algorithms = std::make_shared<CardAlgorithms>(game_config);

 3 Now your lambda captures should work without modification:

 card_algorithms->enumerate_hands(Street::Type(target_street), [=](const std::vector<Card>& hole_cards, const std::vector<Card>& flop) {
     // ...
     card_algorithms->enumerate_street_range([=](const std::vector<Card>& opponent_hole_cards) {
         // ...
     }, Street::Type(0), Street::Type(target_street), reserved_cards);
 });

Using a std::shared_ptr allows multiple owners of the same object, which can be useful in scenarios where you need to share ownership or when you're unsure about the object's lifetime. However, it comes
with a small performance overhead due to reference counting.

If you decide to go this route, make sure to update any other parts of your code that might be expecting a unique_ptr.

Tokens: 22,373 sent, 297 received. Cost: $0.07 request, $0.07 session.
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
src/algorithms/card_algorithms.cpp src/algorithms/card_algorithms.h src/config/config.h src/config/game_config.h src/tests/test_card_algorithms.cpp src/tests/test_wml_rollout_calculator.cpp
> do that

Certainly! I'll modify the HandRolloutTest class to use a std::shared_ptr for card_algorithms. Here are the necessary changes:

src/tests/test_wml_rollout_calculator.cpp

 <<<<<<< SEARCH
     std::unique_ptr<CardAlgorithms> card_algorithms;
 =======
     std::shared_ptr<CardAlgorithms> card_algorithms;
 >>>>>>> REPLACE

src/tests/test_wml_rollout_calculator.cpp

 <<<<<<< SEARCH
         card_algorithms = std::make_unique<CardAlgorithms>(game_config);
 =======
         card_algorithms = std::make_shared<CardAlgorithms>(game_config);
 >>>>>>> REPLACE

These changes will allow you to capture card_algorithms by value in your lambda functions without causing compilation errors. The rest of your code should work as is with this modification.

Tokens: 21,939 sent, 174 received. Cost: $0.07 request, $1.46 session.
Committing src/tests/test_wml_rollout_calculator.cpp before applying edits.
Commit 2aa6a58 feat: Modify the number of cards per street in the WML rollout calculator test
Applied edit to src/tests/test_wml_rollout_calculator.cpp
No changes made to git tracked files.
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
src/algorithms/card_algorithms.cpp src/algorithms/card_algorithms.h src/config/config.h src/config/game_config.h src/tests/test_card_algorithms.cpp src/tests/test_wml_rollout_calculator.cpp
>

Unrelated...Paul is there anything I can do to support the project? I have minimal time to contribute, are you accepting corporate donations?

Version and model info

Aider v0.47.1 Models: claude-3-5-sonnet-20240620 with diff edit format, weak model claude-3-haiku-20240307 Git repo: .git with 1,814 files Warning: For large repos, consider using an .aiderignore file to ignore irrelevant files/dirs. Repo-map: using 1024 tokens

[paul-gauthier]

Thanks for trying aider and filing this issue. This doc may be helpful:

https://aider.chat/docs/troubleshooting/edit-errors.html

It looks like you added a lot of files to the chat?

[paul-gauthier]

I'm going to close this issue for now, but feel free to add a comment here and I will re-open or file a new issue any time.

[aelaguiz]

@paul-gauthier functionally on large projects I am finding myself spending a lot of time managing what is in the chat context.

For me sometimes I get it set up perfectly and then something happens, I lose it, and this can take a while in large projects.

Is there something I'm missing that would allow me to save my context setup, for when I'm working on specific parts of the code base for instance?

[paul-gauthier]

Can you help me understand this better? What does it mean to set up your context? How does this setup differ in different "parts of the code base"?

I'm happy to try and help if I can understand the need better.

[aelaguiz]

Sorry for the slow response here.

So for instance in a code tree that looks like:

• module 1
• module 2
• module 3
• unit tests

If I'm working on unit tests + module 3, I frequently will find that to get the right answers from the LLM it really does need to have the headers from module1 & 2 in the chat context.

So I'll add:

module1/class1.h module1/class2.h module3/my_class.cpp module3/my_class.h unittests/test_my_class.cpp

Obviously quite simplified, but I'm working on an AI system and frequently getting the exact correct files in context is the difference between a nonsensical answer from aider. + sonnet 3.5 or something really helpful.

The process of getting the right items into the chat can be cumbersome and if I accidentally close aider I have to start over.

Inside vs code I actually use different workspace layouts for different parts of project so I can see the things I need to in the right way while I'm coding. That's the analogy I use in my head when describing what I'm doing to set up aider properly.

Is this making any sense, I apologize if it's hard to follow. I'm sure you've got a feature that I'm ignorant of that would make this easier.

[paul-gauthier]

You could launch aider with the right set of files from the command line. And then use your shell history to launch aider with different collections of files?

[paul-gauthier]

I'm going to close this issue for now, but feel free to add a comment here and I will re-open or file a new issue any time.