Refactor component manager

[vasil-pashov]

Reference Issues/PRs

What does this implement or fix?

• Implement perfect forwarding. All entities were passed by value.
• There were two implementations of replace_entities one taking vector and one taking template parameter. There are two issues with this. First we cannot use this to place the same vector in all of the entities because the type deduction will pick up the more specific implementation. Second parameters were passed by value. The function taking vector is now replace_entities_zip and moves the elements of the vector. The function taking a single parameter and placing it in all entities is replace_entities and takes a const ref. We cannot move in this case
• Cosmetic changes: some const std::vector were changed to std::span so that it can accept wider range of types (e.g. std::array)

Any other comments?

Checklist

Checklist for code changes...

• Have you updated the relevant docstrings, documentation and copyright notice?
• Is this contribution tested against all ArcticDB's features?
• Do all exceptions introduced raise appropriate error messages?
• Are API changes highlighted in the PR description?
• Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

[claude]

Critical: this branch is no longer additive — when Args = EntityFetchCount only the std::atomic<EntityFetchCount> is emplaced, and the plain EntityFetchCount component is no longer added. The comment immediately above still documents that both are intentional: the plain copy is the "first-class" entity "accessible by registry_.get<EntityFetchCount>(id), as this is external facing (used by resample)", and the atomic is the internal counter that gets decremented.

External consumers retrieve the plain value via registry_.view<…, const EntityFetchCount>().get(id):

• clause_utils.hpp::gather_entities → get_entities_and_decrement_refcount<…, EntityFetchCount>(...) → the view path.
• ResampleClause::process in clause_resample.cpp:290 is exactly this on production data.
• ComponentManager.Simple (test_component_manager.cpp:35) and ResampleClause tests at test_resample.cpp:375 exercise it directly.

With the plain EntityFetchCount storage now empty, view.get(id) reads from a storage that doesn't contain the entity (UB / assertion in entt). The PR description's reasoning "the EntityFetchCount was always added… which is wrong" doesn't hold here — both entries were required.

The same regression applies to the matching else-branches in add_entities, replace_entities, and replace_entities_zip below. They should remain additive for the EntityFetchCount case (emplace/replace both the plain value and the atomic), not exclusive.

[claude]

std::forward<R>(values)[idx] does not move the element. std::vector::operator[] has no rvalue overload, so even when std::forward<R>(values) produces an rvalue reference to the vector, indexing it returns T& (lvalue) and the elements are copied, not moved. The PR description cites "perfect forwarding" as the motivation, but here it has no effect.

If the intent is to consume the values, use std::move(values[idx]) (or std::ranges::iter_move) inside the loop — keeping in mind that this should only fire when R is an rvalue reference, otherwise it silently moves out of the caller's container.

[claude]

ArcticDB Code Review Summary

Correctness

• ❌ EntityFetchCount plain component no longer stored — add_entity, add_entities, replace_entities, and replace_entities_zip all changed from additive branches to exclusive branches for EntityFetchCount. Only the std::atomic<EntityFetchCount> is now emplaced/replaced; the plain EntityFetchCount component (used by external consumers via registry_.view<…, const EntityFetchCount>().get(id)) is gone. Breaks the ResampleClause::process path (clause_resample.cpp:290) and the existing test_component_manager.cpp / test_resample.cpp tests. The retained comment in component_manager.hpp still documents that both the plain value (external facing) and the atomic (internal-only) are required. See inline comment for full details.

Code Quality

• ❌ Ineffective forwarding in replace_entities_zip — std::forward<R>(values)[idx] always yields an lvalue (std::vector::operator[] has no rvalue overload), so the "perfect forwarding" cited in the PR description doesn't materialize. Use std::move(values[idx]) if the intent is to move-from, gated on whether R is an rvalue reference. See inline comment.

Testing

• ❌ No new tests — the PR description states three functional changes, but no tests were added or modified to demonstrate the bug being fixed (the EntityFetchCount double-storage issue), to exercise the new replace_entities_zip semantics, or to verify the forwarding-reference paths. Per CLAUDE.md, every code change must be accompanied by a failing test that the change fixes. Adding a test would have caught the regression flagged above.