Skip to content

fix(indexers): update from_filespace method to patch batch_size bug#205

Merged
jamie-ons merged 2 commits into
mainfrom
203-fix_vectorstore_from_filespace_bug
Jul 8, 2026
Merged

fix(indexers): update from_filespace method to patch batch_size bug#205
jamie-ons merged 2 commits into
mainfrom
203-fix_vectorstore_from_filespace_bug

Conversation

@jamie-ons

Copy link
Copy Markdown
Contributor

✨ Summary

VectorStore.from_filespace() fails when loading a vector store whose metadata.json has "batch_size": null (as produced by v1.0.0, which did not persist batch_size). This happens when the batch_size argument is None which is the default argument.

This PR adds a hardcoded fallback option of batch_size=128 to avoid the situation where VectorStore created before v1.1.0 is loaded in and fails due to not persisting any batch_size.

Although hardcoding a value is often seen as bad practice, in this case for the edge case where batch_size and metadata["batch_size"] are both None, this may be the cleanest solution. This would mean that a default batch_size would need to be updated in 2 places when maintaining instead of 1.

Other possible solutions would be to:

  1. Split the __init__ method into a __init__ that just sets up the config for the file and a build method. This would be similar to scikitlearn and some other libraries. This would mean we could use the __init__ method when loading from filespace and take the default batch_size. This is quite clean, but a lot of work for such a small issue and goes against our simple one line setup.
  2. Create a constant outside of the class in the main.py file called something like _BATCH_SIZE = 128, then use this in the __init__ and from_filespace methods. This means you only have to update the constant in one place. This doesn't appear very elegant to me though.
  3. Removing the option of not specifying batch_size or setting a default at 128 and not reading it from filespace.
  4. Raise an error and require explicit batch_size for legacy stores if both batch_size and metadata["batch_size"] are None, telling the user their store was created with v1.0.0 and they must pass batch_size explicitly.
  5. Editing the from_filespace method to rewrite the dictionary with a default of 128 if it is None.

From all of these I think the most clean ones are either raising the error and letting the user fix it or rewriting the dictionary to avoid the error in the future.

📜 Changes Introduced

  • Added a hardcoded batch_size to the from_filespace() method in the indexers.main.py file.

✅ Checklist

  • Code passes linting with Ruff
  • DocStrings follow Google-style and are added as per Pylint recommendations
  • Documentation has been updated if needed

🔍 How to Test

On V1.1.0 main, using the DEMO/general_workflow_demo.ipynb as a start we can add the following after creating my_vector_store:

# 1. Create a VectorStore and manually simulate a v1.0.0 metadata file (or any file missing the batch_size)
my_vector_store = VectorStore(
    file_name="data/testdata.csv",
    data_type="csv",
    vectoriser=vectoriser,
    output_dir="testdata",
    overwrite=True,
)

import json
with open("testdata/metadata.json", "r") as f:
    metadata = json.load(f)

metadata["batch_size"] = None  # simulates v1.0.0 metadata

with open("testdata/metadata.json", "w") as f:
    json.dump(metadata, f, indent=2)

# 2. Attempt to reload - raises IndexBuildError
VectorStore.from_filespace(folder_path="testdata", vectoriser=vectoriser)

Confirm this causes an IndexBuildError.

Then change to this branch and repeat the code and confirm it now loads successfully. Please try any other edge cases you think may be possible.

@jamie-ons

jamie-ons commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Overview

After discussing with different team members, we have decided to go with suggested approach 2, with an additional warning explicitly telling users that their metadata file is outdated.

  1. Create a constant outside of the class in the main.py file called something like _BATCH_SIZE = 128, then use this in the init and from_filespace methods. This means you only have to update the constant in one place.

The warnings

When using a VectorStore created using v1.0.0, the batch_size was missing. Now instead of erroring out, we have a warning letting the user know that their VectorStore is outdated and that a default batch_size has been applied if they don't specify one themselves:

if metadata.get("batch_size") is None:
    if batch_size is not None:
        logging.warning(
            "Metadata is outdated (pre v1.1.0) and does not contain a batch_size. Using provided batch_size=%d.",
            batch_size,
        )
    else:
        logging.warning(
            "Metadata is outdated (pre v1.1.0) and does not contain a batch_size. Defaulting to %d.",
            _BATCH_SIZE,
        )

Extra testing

I have additionally tested the from_filespace method more in attempts to make sure there are no other edge cases. My test setup is zipped and attached so it can be verified. To use it, unzip it and create a new conda enviorment then run run.sh using bash.

from_filespace_testing.zip

This mini test project does the following things using pytest

  1. Create VectorStores using v1.0.0, v1.1.0 and v1.1.1 (this pr)
  2. Tests loading the different VectorStores from all previous versions
  3. confirms the bug in v1.1.0
  4. Tests applying your own batch_size
  5. Tests other basic functionality

How to Test

  • Use the provided test suite and verify only the v1.1.0 tests fail.
  • Read the tests and ensure they make sense, please comment any other edge cases you can think of
  • Read the code changes and ensure you think they are a maintainable and functional fix

@jamie-ons
jamie-ons marked this pull request as ready for review July 7, 2026 16:04
@jamie-ons
jamie-ons requested a review from a team as a code owner July 7, 2026 16:04
@lukeroantreeONS lukeroantreeONS changed the title update from_filespace method to patch batch_size bug fix(indexers): update from_filespace method to patch batch_size bug Jul 8, 2026
@github-actions github-actions Bot added the bug Something isn't working label Jul 8, 2026

@lukeroantreeONS lukeroantreeONS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Happy to approve 👍

This is a great PR, with good thought put into the different options, and a sensible final decision & implementation.

Tested using a variety of batch_sizes on creation & re-loading, with VDBs created with v1.0.0 and v1.1.0 and the behaviour is as outlined in the PR, and provides a good user experience.

Image Image Image Image

@jamie-ons
jamie-ons merged commit baf48e6 into main Jul 8, 2026
7 checks passed
@jamie-ons
jamie-ons deleted the 203-fix_vectorstore_from_filespace_bug branch July 8, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VectorStore.from_filespace() raises IndexBuildError when loading a store built with v1.0.0 (missing batch_size in metadata)

2 participants