fix: chunk_by_title() interface is rude

[scanny]

chunk_by_title() interface is "rude"

Executive Summary. Perhaps the most commonly specified option for chunk_by_title() is max_characters (default: 500), which specifies the chunk window size.

When a user specifies this value, they get an error message:

  >>> chunks = chunk_by_title(elements, max_characters=100)
  ValueError: Invalid values for combine_text_under_n_chars, new_after_n_chars, and/or max_characters.

A few of the things that might reasonably pass through a user's mind at such a moment are:

• "Is 110 not a valid value for max_characters? Why would that be?"
• "I didn't specify a value for combine_text_under_n_chars or new_after_n_chars, in fact I don't know what they are because I haven't studied the documentation and would prefer not to; I just want smaller chunks! How could I supply an invalid value when I haven't supplied any value at all for these?"
• "Which of these values is the problem? Why are you making me figure that out for myself? I'm sure the code knows which one is not valid, why doesn't it share that information with me? I'm busy here!"

In this particular case, the problem is that combine_text_under_n_chars (defaults to 500) is greater than max_characters, which means it would never take effect (which is actually not a problem in itself).

To fix this, once figuring out that was the problem, probably after opening an issue and maybe reading the source code, the user would need to specify:

>>> chunks = chunk_by_title(
...     elements, max_characters=100, combine_text_under_n_chars=100
... )

This and other stressful user scenarios can be remedied by:

• Using "active" defaults for the combine_text_under_n_chars and new_after_n_chars options.
• Providing a specific error message for each way a constraint may be violated, such that direction to remedy the problem is immediately clear to the user.

An active default is for example:

• Make the default for combine_text_under_n_chars: int | None = None such that the code can detect when it has not been specified.
• When not specified, set its value to max_characters, the same as its current (static) default.
  This particular change would avoid the behavior in the motivating example above.

Another alternative for this argument is simply:

combine_text_under_n_chars = min(max_characters, combine_text_under_n_chars)

Fix

1. Add constraint-specific error messages.
2. Use "active" defaults for combine_text_under_n_ chars and new_after_n_chars.
3. Improve docstring to describe active defaults, and explain other argument behaviors, in particular identifying suppression options like combine_text_under_n_chars = 0 to disable chunk combining.

[shreyanid]

Looking at the chunking documentation and the docstring of chunk_by_title in this PR, I'm trying to understand the relationship between these 3 parameters combine_text_under_n_chars, new_after_n_chars, and max_characters

• new_after_n_chars is after how many characters a section should be capped and a new section should start. Can this value in theory be greater than max_characters?
• combine_text_under_n_chars is if a section has under this many characters, it will be combined with another(?) section
• max_characters (not mentioned in the documentation) is to specify a "chunk size"? for elements text specifically? - im not sure how this hard max and the soft max new_after_n_chars act differently.

I am very new to chunking/RAG but I can review given a little context/crash course on how each of these are used :)

[shreyanid]

can we add a test for capping new_after_n_chars at max_characters? and any other cases here in this old test that aren't already covered

[scanny]

Sure, added a test for that.

[scanny]

All the cases from the old test were already covered.

[qued]

As far as I can tell, this syntax wasn't introduced until Python 3.10. On the other hand, CI isn't complaining (maybe because typing isn't really enforced?). Everywhere else we're using the Optional[int] syntax, I'm inclined to stick with that here.

[scanny]

This syntax is accepted in Python 3.7+ (and maybe further back) when from __future__ import annotations appears in the import section.

This "pipe == Union" syntax is recommended as Union will be deprecated, so it's the "way forward" so to speak, which is why I started using it. Let me see if I can find a reference for that ... here's one: https://mail.python.org/archives/list/[email protected]/message/624BEVFNZXFEQP6ZWIGB4YKSPMCBE2M6/

next response in that thread is Guido approving.

The from __future__ import annotations also gives some other very nice benefits like not requiring forward references to be quoted and a few other things.

[scanny]

Ok, fix is in for this @qued :)