Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/semantic/filtered_ast_visitor.h (1)

36-52: Stale comment and redundant branch after the simplification.

Two small cleanups are worth doing here:

1. The comment on lines 36–37 ("Clang doesn't visit implicit declarations and implicit template instantiations by default") used to justify the explicit skip logic that has now been removed. In its current position it sits above the TranslationUnitDecl branch, which is unrelated — it reads as explaining the TU handling. Either move it back to where the implicit-decl filtering reasoning applies (or drop it).
2. Once interested_only is false, both branches do exactly return Base::TraverseDecl(decl);, so the outer isa<TranslationUnitDecl> block can collapse into a single early return guarded by interested_only.

♻️ Suggested tightening

-        /// Clang doesn't visit implicit declarations and
-        /// implicit template instantiations by default.
-        if(llvm::isa<clang::TranslationUnitDecl>(decl)) {
-            if(interested_only) {
-                for(auto top: unit.top_level_decls()) {
-                    if(!TraverseDecl(top)) {
-                        return false;
-                    }
-                }
-                return true;
-            }
-
-            return Base::TraverseDecl(decl);
-        }
-
-        return Base::TraverseDecl(decl);
+        /// When `interested_only` is set, only walk the top-level decls of the
+        /// interested file instead of the full translation unit. Implicit decls
+        /// and implicit template instantiations are skipped by
+        /// `RecursiveASTVisitor` by default, so no extra filtering is needed.
+        if(interested_only && llvm::isa<clang::TranslationUnitDecl>(decl)) {
+            for(auto top: unit.top_level_decls()) {
+                if(!TraverseDecl(top)) {
+                    return false;
+                }
+            }
+            return true;
+        }
+
+        return Base::TraverseDecl(decl);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/semantic/filtered_ast_visitor.h` around lines 36 - 52, The comment about
implicit declarations is stale and the TranslationUnitDecl branch has redundant
returns; remove or relocate the comment (it belongs with the implicit-decl
filtering logic, not above the TranslationUnitDecl handling), then simplify the
control flow: if interested_only is true, iterate unit.top_level_decls() and
call TraverseDecl(top) (return false on failure and true after the loop),
otherwise just call and return Base::TraverseDecl(decl); eliminate the
unnecessary isa<clang::TranslationUnitDecl> outer branch so TranslationUnitDecl
handling only appears as the interested_only early-path and otherwise falls
through to Base::TraverseDecl.

src/feature/document_symbols.cpp (1)

115-145: push_symbol relies on guaranteed copy elision of Guard — worth a quick defensive tweak.

The pattern returns a locally-defined Guard by value from push_symbol and captures it with auto guard = this->push_symbol(decl);. C++17 mandatory prvalue copy elision makes this sound today, and the user-provided destructor suppresses the implicit move so any accidental future refactor that forces a copy (e.g., binding through a function-like wrapper, storing in a variant) would invoke the implicit copy constructor and run the source Guard’s destructor prematurely, restoring result.cursor before traversal of children completes.

Consider either marking Guard as move-only (and defaulting the move constructor while deleting copy), or clearing previous in the moved-from guard. Low priority — current code is correct.

♻️ Defensive hardening

         struct Guard {
             std::vector<InternalSymbol>* previous;
             std::vector<InternalSymbol>** cursor;
 
+            Guard(const Guard&) = delete;
+            Guard& operator=(const Guard&) = delete;
+            Guard(Guard&& other) noexcept
+                : previous(std::exchange(other.previous, nullptr)),
+                  cursor(other.cursor) {}
+            Guard& operator=(Guard&&) = delete;
+
             ~Guard() {
                 if(previous) {
                     *cursor = previous;
                 }
             }
         };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/feature/document_symbols.cpp` around lines 115 - 145, push_symbol returns
a local Guard with a non-trivial destructor which can be accidentally copied and
prematurely restore result.cursor; make Guard move-only to prevent accidental
copies by deleting its copy constructor/assignment and defaulting the move
ctor/assign (or alternatively ensure moved-from Guard clears previous so its
destructor is no-op). Update the inner struct Guard in push_symbol: delete copy
ctor/assign and default move ctor/assign (or clear previous on move) and keep
the destructor logic to restore result.cursor from previous.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes