fix(batch): key wholesale-failure object errors by original index

[feiiiiii5]

Problem

Client-side batching reports fewer object failures than actually occur, and can attribute them
to the wrong input rows.

The wholesale-failure handler in _BatchBase.__send_batch keyed its error map by the position
inside the chunk being sent:

errors_obj = {idx: ErrorObject(message=repr(e), object_=obj) for idx, obj in enumerate(objs)}

but BatchObjectReturn.__add__ merges chunk results with a plain dict.update and does no
re-keying (classes/batch.py:231). With batch_size smaller than the number of objects, every
chunk contributes keys 0..len(chunk)-1, so later chunks overwrite earlier ones.

This contradicts the documented contract on BatchObjectReturn
("The keys of the errors and uuids dictionaries will always be equivalent to the
original_index of the objects as you added them to the batching loop") and every other failure
site in the same flow, which already uses the global index (grpc_batch.py:157, sync.py:367,
sync.py:410, async_.py:409, async_.py:452).

Observed on main @ 142d798, four objects with fixed_size(batch_size=2), both chunks rejected:

result.errors keys : [0, 1]        # but 4 objects failed
len(failed_objects): 4
errors[0] -> object_.index=2
errors[1] -> object_.index=3

The sharper variant is a successful chunk followed by a wholesale failure: uuids keys [0, 1]
and errors keys [0, 1] end up describing different objects, so a caller that retries
"the failed indices" re-sends rows that already landed and skips rows that did not.

Fixes #2165.

Change

-                errors_obj = {
-                    idx: ErrorObject(message=repr(e), object_=obj) for idx, obj in enumerate(objs)
-                }
+                errors_obj = {obj.index: ErrorObject(message=repr(e), object_=obj) for obj in objs}

BatchObject.index is a required int assigned once in _add_object (base.py:826) and is not
reset by flush(), so it is always present and stable across retries. Chunks are popped under
self.__lock, so no object can appear in two in-flight chunks and produce a duplicate key.

Why the references sibling is untouched

The next handler down, errors_ref = {idx: ... for idx, ref in enumerate(refs)}, looks like the
same bug and I deliberately left it alone. BatchReferenceReturn.__add__ merges with an offset
(classes/batch.py:293-299):

prev_max = max(self.errors.keys()) if len(self.errors) > 0 else -1
for key, value in other.errors.items():
    self.errors[prev_max + key + 1] = value

That arithmetic expects dense, chunk-local keys: two chunks of two currently produce
{0: ref0, 1: ref1, 2: ref2, 3: ref3}. Switching it to ref.index would yield {0, 1, 4, 5} —
indices 2 and 3 would vanish and keys past the end would appear. Verified by running the two
merge paths. References do have an index problem, but it comes from elsewhere (the REST
references path returns sparse chunk-local keys, and the non-wholesale handlers feed
already-global keys into this offset merger), so it needs its own change rather than a symmetric
edit here.

Verification

One venv, Python 3.11.15, -e . plus requirements-devel.txt and requirements-test.txt; every
command run identically on main @ 142d798 as a control.

• python -m pytest mock_tests/ test/ -q
  • base: 2 failed, 556 passed, 1 skipped — both failures are the new tests
  • this branch: 558 passed, 1 skipped
• The new mock_tests/test_batch_error_indices.py drives the real client against a mock gRPC
  service, no network and no server:
  • test_wholesale_failed_chunks_report_every_object — asserts all four indices appear and
    that each key describes the object it names, plus len(failed_objects) == len(errors)
  • test_failed_and_successful_chunks_do_not_share_keys — the uuid/error overlap above
  • test_successful_chunks_keep_original_indices — passes on base too; it pins the convention
    the fix aligns to rather than testing the fix
• python -m ruff check weaviate test mock_tests → all checks passed
• python -m ruff format --check weaviate test mock_tests → 421 files already formatted
  (420 on base, so the new file is clean under the CI command at .github/workflows/main.yaml:50)
• python -m flake8 on both changed files → clean
• Not run here: integration/, embedded/, journey/ and test-package need Docker and a real
  Weaviate; pyright is not installed in this venv. A repo-wide grep found no test asserting the
  old chunk-local behaviour, but those gates are CI's to judge.

One known knock-on, left alone deliberately

base.py:716-720 filters the deprecated all_responses list by position
(enumerate(all_responses)) against readded_objects, which holds global-index objects. Under
the old keys that filter matched by accident; with correct keys a retried chunk keeps its
re-added objects instead of dropping them. That attribute is deprecated and emits a warning on
access, and the same mismatch already exists on the main grpc_batch.py path (:143 is
position-indexed while :157 uses obj.index), so widening scope here would be a second,
separate change. Flagging it so it is not mistaken for an oversight.

---

Disclosure: prepared with an AI coding assistant. Every number above comes from a command run
against main @ 142d798, repeated on the unpatched tree with the identical command line so the
delta is checkable. The references analysis in particular was checked by executing both merge
strategies, not by reading them.

[orca-security-eu]

Orca Security Scan Summary

Status	Check	Issues by priority
Passed	Infrastructure as Code	0   0   0   0	View in Orca
Passed	SAST	0   0   0   0	View in Orca
Passed	Secrets	0   0   0   0	View in Orca
Passed	Vulnerabilities	0   0   0   0	View in Orca

[weaviate-git-bot]

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

[feiiiiii5]

I have read and I agree with the Contributor License Agreement.

[feiiiiii5]

Hi, CI is green (unit tests + lint on the branch) and the CLA comment is on this PR. The failure being fixed: batch.aggregate keyed wholesale-failure error objects to sequential positions instead of the original request indexes, so per-item errors could be attributed to the wrong request. One small unit test added; nothing else touched. Anything blocking a review?

[feiiiiii5]

This PR already is the construction-side fix: base.py:649 is the only site that builds object-error keys from enumerate(objs), and keying it by obj.index means no chunk-local object-error map is ever built.

Offsetting at the reader can't work for objects. Three producers already key globally and BatchObjectReturn.__add__ is a plain dict.update, which is what makes them correct: grpc_batch.py:157 maps the server's chunk-local index to obj.index, sync.py:367 and async_.py:418 key by cached.index. An offset in __add__ would renumber those. The BatchObjectReturn docstring also states the keys are the original index, so the reader is right to leave them alone.

On the other helpers with the same shape: after this change there are none for objects. For references the chunk-local shape is deliberate, because BatchReferenceReturn.__add__ offsets precisely since rest.py:41 and base.py:769 are chunk-local. That merger is also handed already-global keys by sync.py:389 and async_.py:439 though, which looks like a separate defect with its own cause. Happy to put that one up on its own if you want it looked at.