[jhonabreul]

Description

A key built from a user-facing name fails only at the end of the run, in OnEndOfAlgorithm, with LocalObjectStore: path is not supported: 'trade_log_ai_hardware_&_cloud.csv'. The output of an otherwise green backtest is lost.

The key rules:

• A key may contain english letters, numbers, spaces and ., /, \, _, #, -, $, =. Dots are allowed anywhere; .. never is, since it would leave the storage root.
• \ stays a separator, as before this PR: the local store reads it as /, so folder\results.csv and folder/results.csv are one object.
• ObjectStore saves an unsupported key under its sanitized name instead of rejecting it: every unsupported character and every . of a .. run become _.
• That save replaces whatever the sanitized name holds, in this run or a later one. The store keeps no record of who wrote an object.
• Read, contains, delete and file-path calls use the sanitized name when it exists, else an object stored under the name as given, else the sanitized name.
• ObjectStore.Keys lists an unsupported key saved in the run next to its sanitized name, while that object exists.
• Keys.Contains, and Python's in on the keys, find a key the way ContainsKey does, with one lookup and no listing.
• Keys lists the store only when it is first enumerated, counted or copied, once per returned collection.
• One warning per store, on the first sanitized key, through QCAlgorithm.Debug (the engine log in QuantBook). It names the issues, the rules and the key used.
• A key with no name, such as /, cannot be sanitized: a save or a read through it throws.
• LocalObjectStore.SaveBytes keeps rejecting unsupported keys, as the backstop for callers that bypass the wrapper. The rule helpers are private to ObjectStore.

Decided by a QuantConnect member, to keep the store simple:

• Two unsupported keys that sanitize alike, such as p&q.csv and p:q.csv, share one object and replace each other. The warning tells the user which key is used.
• An object listed under two names shows twice when iterating Keys. Enumerating the store itself yields it once.
• A Keys collection held in a variable shows the store as of its first enumeration, not as of the Keys call.

Warning shape:

ObjectStore: key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. Keys may only contain english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'. Using 'trade log_ AI _ Cloud.csv' instead. Any other unsupported key is sanitized the same way

Builds on #9905, already merged: PythonObjectStore forwards the warning callback, so a Python algorithm's key warnings reach its debug log, and its save helpers follow the same rules.

Related Issue

N/A

Motivation and Context

A completed backtest should not die on its final persistence call over a key spelling.

Requires Documentation Change

Yes: the key rules, how an unsupported key is sanitized and listed, and how existing objects under unsupported names behave.

How Has This Been Tested?

• LocalObjectStoreTests: sanitization on save and the reads that follow, dots kept in folder names and multiple extensions, the once-per-store warning text, the issue wording per case, empty and nameless keys.
• LocalObjectStoreTests: traversal keys (../x, ../../x, x/../y, x/../../y, with \ too) stored with no .. left and their file paths inside the storage root.
• LocalObjectStoreTests: a save through an unsupported key replacing the object under its sanitized name, in the same run and in a new run.
• LocalObjectStoreTests: GetFilePath of an unsupported key returning the file the save that follows reads.
• LocalObjectStoreTests: existing unsupported keys read as given, also as custom data; a save to such a key going to the sanitized name, which reads then return.
• LocalObjectStoreTests: Keys holding both names once each after a save, the unsupported one leaving with the object, and Contains matching ContainsKey. The same from Python with in and list().
• LocalObjectStoreTests: Contains from C# and in from Python causing no folder listing, enumeration listing once per collection, and a new Keys access seeing a later save.
• LocalObjectStoreTests: the LocalObjectStore backstop; QCAlgorithm warning through Debug and QuantBook through the engine log.
• LocalObjectStoreTests: a \ key saved in two runs and read back by both spellings, with no warning.
• LocalObjectStoreTests: the Python helpers save and read an unsupported key under its sanitized name.
• AlgorithmPythonWrapperTests: a Python algorithm's key warning lands in its debug log.
• ObjectStoreKeySanitizationRegressionAlgorithm (C# and Python): the unsupported key is stored, listed and read back under both names, updated, and replaces an object written directly under the sanitized name.
• Custom data object store regression algorithms, QuantBookTests and CustomLiveDataFeedTests.
• All of the above in one run: 351 passed, 0 failed.
• Not repeated for this revision: the two-backtest run through the Launcher. A new store on the same folder stands in for the second run in the tests above.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Refactor (non-breaking change which improves implementation)
• Performance (non-breaking change which improves performance. Please add associated performance test and results)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Non-functional change (xml comments/documentation/etc)

Checklist:

• My code follows the code style of this project.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

[jhonabreul]

Rebased on master and updated. The newest changes:

• Rebase. The conflicts with Improve ObjectStore ergonomics for end-of-run persistence #9905 are resolved: QCAlgorithm.SetObjectStore keeps an already wrapped store and passes Debug, and QuantBook keeps building the Python store for Python notebooks.
• PythonObjectStore. It takes the warning callback, so a Python algorithm's key warnings reach its debug log. Its save helpers carry the key rules on their path docs.
• Dots. A key may carry dots anywhere, in folder names and multiple extensions. Only .. is unsupported, since it would leave the storage root, plus a key that is nothing but dots and separators.
• Backslash. \ is no longer a supported character. It sanitizes to /, the separator the local store already read it as.
• GetFilePath fix. LocalObjectStore.GetFilePath registers a missing key, so getting the path of an unsupported key and then saving it threw the collision error. The key is now paired with its sanitized name when that call creates the entry.
• One regex. The supported check reuses the unsupported-characters regex, so the character set is written once. A key ending in a newline is no longer accepted, which the anchored regex allowed.
• Tests. Traversal keys (../x, ../../x, x/../y, x/../../y, with \ too) end with no .. and a file path inside the storage root. GetFilePath then Save, nameless keys, dotted keys and the Python helpers with an unsupported key are covered too.
• Result. Object store, Python wrapper, QuantBook, custom data feed and object store regression tests on the rebased branch: 217 passed, 0 failed.

The description is updated to match.

[quantconnect-kronos-reviewer]

Requesting changes: one regression for keys that work today, a design question on the main use case that needs a person to decide, and a missing end-to-end run.

CI is green on 5c9c02a.

1. A key with \ that works on master throws from the second run on (blocks). This PR now treats \ as unsupported. Before, the local store accepted it and stored it as /. Now it goes through the sanitized-save path, and its collision rule throws once the / name already exists, which it does after the first run. So an algorithm that saves ObjectStore.Save("folder\\results.csv", ...) every backtest works on master and throws ArgumentException here from the second backtest on. Reproduced locally:

Save("kronos_folder\\results.csv", "run1")   // fresh store, run 1: ok
Save("kronos_folder\\results.csv", "run2")   // fresh store, run 2:
ArgumentException: ObjectStore: key 'kronos_folder\results.csv' is not supported and its sanitized name 'kronos_folder/results.csv' already holds another object. ...

The same test passes on master. LocalObjectStore.SaveBytes now also rejects \ for callers that bypass the wrapper (ObjectStore.ValidateKey). The store has always read \ and / as the same object, so the collision rule doesn't protect anything there. A smaller fix: turn \ into / before the supported check, so it never enters the collision path and the backstop accepts it as before. Please add a test that saves a \ key in two runs.

2. The case in the description fails again on the next backtest (for a person to decide). The motivation is that "a completed backtest should not die on its final persistence call over a key spelling". With this change, Save("trade_log_ai_hardware_&_cloud.csv", ...) in OnEndOfAlgorithm succeeds on the first backtest of a project. Every later backtest, and a live algorithm after a redeploy, throws at that same call, because the sanitized object came from an earlier run (ANewRunThrowsOnSavingAnUnsupportedKeyAPreviousRunSanitized covers this on purpose). Users rerun backtests all the time. On master the failure shows up on the first run. Here it waits until the user is relying on the output. Choosing between "never overwrite an object this run didn't write" and "an unsupported key reliably persists across runs" is a product decision, so I'm asking a QuantConnect member to make it on this PR. Possible options: let a save overwrite the sanitized name and warn every time, or keep rejecting unsupported keys with the clearer per-issue message and leave sanitizing out.

3. ContainsKey and the other reads now throw for a key with no name (minor). ContainsKey("/"), or a key made only of dots and separators or with more than 100 /, returned false on master and now throws ArgumentException from SanitizeKey, via KeyToRead. Reproduced locally; it passes on master. This is a new way for a check-before-read to fail. On the read path, fall back to the key as given when it can't be sanitized.

Testing evidence. The unit and regression tests are thorough on the paths they cover. Two gaps: no test saves a \ key in two runs, which is why (1) slipped through, and there's no end-to-end run. This changes how algorithms persist data, so please add an end-to-end outcome to the description: a project backtested twice that saves an unsupported key and a \ key in OnEndOfAlgorithm, and what each run did.

Other repositories. QuantConnect/lean-cli#698 prints the warnings for object-store set and stays consistent with this. The Documentation repository needs the key rules and collision behavior, as the description says. The new optional warn parameter on ObjectStore/PythonObjectStore reaches the generated stubs on release.

Risk label: risk:high. This changes the user-facing ObjectStore API: what saves accept and when they throw, plus a new constructor parameter. Item (1) breaks algorithms that work today. To lower it: fix (1) and (3), get a decision on (2), and add an end-to-end run.

[quantconnect-kronos-reviewer]

\ isn't in this set any more, so a key the local store accepted on master (folder\results.csv, stored as folder/results.csv) now goes through KeyToSave's collision check and throws from the second run on. LocalObjectStore.SaveBytes also rejects it now through ValidateKey. Turning \ into / before the supported check would keep today's behavior. See (1) in the review.

[quantconnect-kronos-reviewer]

_savedKeys starts empty in every run, so the sanitized object an earlier backtest saved through this same key makes every later backtest throw here at the same OnEndOfAlgorithm save the description wants to fix. See (2) in the review, which needs a QuantConnect member's decision.

[quantconnect-kronos-reviewer]

SanitizeKey throws when there is no valid key to sanitize to ("/", "./", >100 /), so ContainsKey, ReadBytes, Delete and GetFilePath throw ArgumentException here where master returned false or not-found. Reproduced with ContainsKey("/"). On the read path, falling back to path when sanitizing fails would keep master's behavior.

[jhonabreul]

Thanks for the review. Finding 1 is fixed in e2b689d, and findings 2 and 3 were decided by a QuantConnect member, with no code change.

1. A \ key threw from the second run on. Fixed. \ is an accepted separator again and is passed to the store unchanged, as on master, so it never enters the sanitized-save path and LocalObjectStore.SaveBytes accepts it as before. The rules text and the path docs list it again.

• Test added, as asked: ABackslashKeyIsSavedInEveryRunAsOnTheStoreItAlwaysReadAsASeparator saves backslash_folder\results.csv in two runs and reads it by both spellings, with no warning.
• It fails on 5c9c02a with the ArgumentException you reproduced, and passes now.

2. A later run throws for the same unsupported key. Decided: keep it. The first run's warning names the sanitized key and says a save throws when that name already holds an object not written from that key in this run. So it is on the user to switch to the supported key. The rule stays that an object is only replaced through its exact key. The description records the decision.

3. Reads throw for a key with no name. Accepted as is. A key such as / or ./ cannot name an object, and a read fails with the same message a save gives. No change.

End-to-end run, as asked. One algorithm backtested twice through the Launcher against the same object store folder, saving both keys in OnEndOfAlgorithm:

	e2e_folder\results.csv	trade_log_ai_hardware_&_cloud.csv
Run 1	Saved, read back as e2e_folder/results.csv	Saved as trade_log_ai_hardware___cloud.csv, with the warning in the debug log
Run 2	Saved again, content updated	ArgumentException at the save, reported as a runtime error; the stored object kept run 1's content

Run 2's error is the behavior decided in finding 2. The outcome is in the description.

Result. Object store, Python wrapper, QuantBook, custom data feed and object store regression tests: 216 passed, 0 failed.

Other repositories. QuantConnect/lean-cli#698 is unaffected. The Documentation change stays listed as required in the description.

[jhonabreul]

Pushed e09336f76, which simplifies how a sanitized save behaves. The description is updated to match.

What changed:

• The store no longer tracks which key wrote each sanitized name. The map, the collision ArgumentException and the pairing in GetFilePath are gone.
• A save through an unsupported key always goes to its sanitized name and replaces the object there, in the same run or a later one.
• Reads use the sanitized name when it exists, else an object stored under the name as given, else the sanitized name.
• ObjectStore.Keys now lists an unsupported key saved in the run next to its sanitized name, while that object exists.
• Keys.Contains, and Python's in on the keys, now agree with ContainsKey. Before, ContainsKey(key) was true after a sanitized save while Keys.Contains(key) was false.

Why: the collision rules needed state and several special cases, and a later run threw on the key the first run had sanitized. The user is warned with the key used, so keeping keys apart is on them.

What to know when reviewing:

• Two unsupported keys that sanitize alike now replace each other silently, after the one warning per store.
• An object saved through an unsupported key shows under both names when iterating Keys. Enumerating the store yields it once.
• For Python's in, the keys collection exposes ContainsKey next to Count, which is what the Python runtime looks for before it falls back to iterating.

Tests: the collision tests are replaced by replace-on-save, new-run and Keys tests, in C# and Python. 350 passed, 0 failed across the object store, storage, QuantBook, AlgorithmPythonWrapperTests and CustomLiveDataFeedTests suites, both regression algorithms included.

[jhonabreul]

Pushed e24e6d004, which makes ObjectStore.Keys list the store only when needed. The description is updated to match.

What changed:

• Keys returns at once. The store is listed when the collection is first enumerated, counted or copied, once per returned collection.
• Keys.Contains, and Python's in on the keys, do one ContainsKey lookup and never list. On master Keys.Contains lists the whole storage folder.
• With no sanitized key in the run, the usual case, the listing is the store's own keys as they are, with nothing merged in.

What to know when reviewing:

• A Keys collection held in a variable now shows the store as of its first enumeration, not as of the Keys call. A new Keys access lists again.
• Nothing is cached across accesses, so a listing cannot go stale when files change around the store.

Test: KeysListTheStoreOnFirstUseOnlyAndContainsNeverDoes counts the folder listings through a file handler. It failed before the change with three listings from three Keys accesses that nothing enumerated. 351 passed, 0 failed across the suites named in the description.

Since the last review, the two commits to look at are e09336f76 (no saved-keys state, replace on save, Keys lists both names) and this one.

[quantconnect-kronos-reviewer]

Approving e24e6d0. My earlier findings are resolved or decided, CI is green on this commit, and I ran it end to end myself.

Earlier findings.

1. A \ key threw from the second run on: fixed in e2b689d. \ is a separator again and is passed to the store unchanged. ABackslashKeyIsSavedInEveryRunAsOnTheStoreItAlwaysReadAsASeparator covers it.
2. A later run threw for the same unsupported key: resolved by e09336f. A save always goes to the sanitized name and replaces whatever is there. As the description records, a QuantConnect member decided this, along with keys that sanitize alike sharing one object. A member also approved that commit.
3. Reads throw for a key with no name, such as /: kept as is, and the description now states it. I'm not raising it again.

What changed since my last review.

• e09336f drops the per-run collision state. Reads prefer the sanitized name, then fall back to an object stored under the name as given. Keys lists an unsupported key saved in this run next to its sanitized name, but only while that object exists. Keys.Contains now agrees with ContainsKey.
• e24e6d0 makes Keys lazy. The listing still comes from LocalObjectStore.Keys, a fresh list, so deleting while iterating stays safe. Contains and Python's in do a single ContainsKey lookup. The description lists the one behavior change: a Keys value held in a variable now reflects the store at its first enumeration.
• LocalObjectStore.SaveBytes now calls ObjectStore.ValidateKey. It accepts everything the old regex did, keeps rejecting the rest with the per-issue message, and still caps separators at 100.

End-to-end run. I built this commit and ran one Python algorithm as two backtests through the Launcher against one object store folder. In on_end_of_algorithm it saves trade_log_ai_hardware_&_cloud.csv and e2e_folder\results.csv, then reads them back.

• Run 1: both saves returned True. The warning above reached the algorithm's debug output. Both keys and the sanitized// spellings read back the saved content. in and len on object_store.keys worked, and the keys listed e2e_folder/results.csv, the unsupported key and its sanitized name.
• Run 2: both keys first read back run 1's content. Both saves then returned True and replaced it with run 2's. The same three keys were listed. The folder held only e2e_folder/results.csv and trade_log_ai_hardware___cloud.csv.

That is the outcome the description promises: the end-of-run save no longer fails, in the first run or any later one.

Other repositories. The Documentation repository still needs the key rules, how keys are sanitized and listed, and how existing objects under unsupported names behave, as the description says. QuantConnect/lean-cli#698 stays consistent with this. The new optional warn parameter on ObjectStore and PythonObjectStore reaches the stubs on the next release.

Risk label: risk:high, unchanged. This changes user-facing ObjectStore API and behavior: which keys a save accepts and where it stores them, what Keys returns and when it lists, and a new constructor parameter. The evidence is strong: unit tests, regression algorithms in C# and Python, green CI and the end-to-end run. But a change this wide to how algorithms persist data doesn't qualify for risk:low.