[jhonabreul]

lean cloud object-store set sends the key exactly as typed. QuantConnect/Lean#10029 changes LEAN's ObjectStore to save an unsupported key (anything beyond english letters, numbers, spaces, /, \, _, #, -, $, = and at most one .extension) under its sanitized name. Without the same rule here, a key set from the CLI as trade log: AI & Cloud.csv is not the key an algorithm saves it under.

The change:

• set sanitizes an unsupported key the way LEAN's save does, with a warning naming the issues and the key to use from then on. When the sanitized key already holds an object the command fails before any set request, as LEAN's save throws: nothing records who wrote that object, so it is only replaced through its exact key (ObjectStoreClient.exists, one properties request made only for unsupported keys).
• get, delete, list and properties keep sending the key as typed. Only the save mirrors LEAN: its reads also fall back to the sanitized key when the typed one is missing, the CLI does not, the set warning names the key to use.
• The rules live in object_store_helper (key_to_save, get_key_issues, sanitize_key), with the same regexes and wording as LEAN.
• The set help states the behavior and that the other commands take the key as typed, with the README regenerated.

Warning and error shape on set:

Key 'report (1).csv' is not supported: unsupported characters '(', ')'. Keys may only contain english letters, numbers, spaces and the characters '/', '', '_', '#', '-', '$' and '=', plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'. Using 'report _1_.csv' instead
Error: Key 'report (1).csv' is not supported and its sanitized name 'report _1_.csv' already holds another object. Delete 'report _1_.csv' or set it directly to replace it

Tests:

• tests/components/util/test_object_store_helper.py: the issue wording per case, the sanitized key per case, unfixable keys raising, key_to_save raising when the sanitized key already holds an object.
• tests/components/api/test_object_store_client.py: exists on a found object, on the api's exact File not found <key> message and on another failure.
• tests/commands/cloud/object_store: set sanitizing, failing without a request when the sanitized key is taken, and making no request for a supported key; get, delete, list and properties sending unsupported keys as typed with no request.
• Full unit suite and static_analysis.py pass.

End to end against a cloud object store, with throwaway keys under an unused prefix:

• set "report (1).csv" stored report _1_.csv with the warning; the same set again exited 1 with the error and wrote nothing; set "report _1_.csv" replaced it directly.
• model.v2.pkl stored as typed through the api next to model_v2.pkl: properties, get and delete "model.v2.pkl" acted on the typed object only, with no warning.
• Every test object was removed and confirmed gone.

Pairs with QuantConnect/Lean#10029, which defines the rules and merges first; the merge order is handled separately and does not affect the review.

[quantconnect-kronos-reviewer]

Requesting changes. Unsanitized keys in get, delete, list and properties now point at a different object than the one the user named. The PR's premise about LEAN also doesn't match LEAN's code.

1. Read and delete commands act on a different object (blocking: data loss)
The cloud object store accepts any key: the set endpoint applies no key format, and the CLI before this PR sent keys as typed. So keys like model.v2.pkl, data/2024.01.csv or report (1).csv can exist today. With this PR, delete "model.v2.pkl" warns, then deletes model_v2.pkl, which is another object if one exists. I reproduced this end to end:

• stored model.v2.pkl (2 bytes) and model_v2.pkl (5 bytes)
• ran this branch's lean cloud object-store delete "model.v2.pkl", which printed the warning and Using 'model_v2.pkl' instead
• afterwards, properties model_v2.pkl returned File not found and model.v2.pkl was still there

The CLI can no longer delete, get or inspect model.v2.pkl at all. get and properties fetch the wrong object's data in the same way, and list lists the wrong root.

2. set can overwrite another object
set "report (1).csv" writes to report _1_.csv and silently replaces whatever is stored there. The warning prints, but the overwrite has already happened. The helper tests include a collision case, but nothing guards against it.

3. The premise doesn't match LEAN
The description says LEAN's ObjectStore "sanitizes anything else", and the code comment at object_store_helper.py:20 says the same. LEAN doesn't sanitize. LocalObjectStore.SaveBytes checks the same regex and the 100-separator limit, then throws ArgumentException("path is not supported"). ReadBytes, ContainsKey and Delete don't check the key format at all: they only normalize it (trim leading . and / or \, map \ to /). An algorithm can therefore read a key like trade log: AI & Cloud.csv by that exact name. Mirroring LEAN would mean:

• leaving keys unchanged on get, delete, list and properties
• for set, either rejecting the key with the issues message (as LEAN's save does) or keeping it and warning. Renaming it isn't what LEAN does.

There's no linked issue showing the failure this PR fixes. Please link one, or describe the failure, so the scope can be checked against it.

4. Testing

• The unit tests are thorough for the helper's wording and the sanitization. They don't cover the failure modes above: an existing key that doesn't match the format and goes through get, delete, properties or list, or a set that collides with another key.
• The description has no end-to-end run against the cloud object store. Please add the outcome of one that exercises keys that don't match the format.

Minor

• _find_key_issues falls back to "it does not follow the key format". Once the cases above are handled, check whether anything can still reach that fallback.

CI: 34 checks pass. One duplicate windows-latest, 3.14 job was still pending when I submitted; the same job passed in the other run.

Other repositories: none need an update. The api accepts any key and LEAN is unchanged.

Risk: I set risk:high. This is a user-facing behavior change across five commands, and in its current form it can delete or overwrite the wrong object.

[quantconnect-kronos-reviewer]

This deletes the sanitized key, not the one the user typed. If both model.v2.pkl and model_v2.pkl exist, delete "model.v2.pkl" removes model_v2.pkl (reproduced end to end). Please send the key as typed: LEAN's Delete doesn't check the key format.

[quantconnect-kronos-reviewer]

Same problem as delete: an existing key that doesn't match the format can no longer be downloaded, and the sanitized name may return another object's data.

[quantconnect-kronos-reviewer]

Renaming on set can silently overwrite another object, e.g. report (1).csv becomes report _1_.csv. LEAN's SaveBytes rejects such a key and doesn't rename it. Rejecting here with the issues message would match LEAN and couldn't overwrite anything.

[quantconnect-kronos-reviewer]

LEAN doesn't sanitize keys. LocalObjectStore.SaveBytes throws ArgumentException for a key that fails this regex, and reads and deletes skip the check entirely.

[jhonabreul]

Thanks, the reproduction was right: the first version rewrote keys on every command. Reworked as you suggested, each point confirmed with a failing test first.

• Premise (3): the PR mirrors LEAN as changed by Improve ObjectStore ergonomics for end-of-run persistence Lean#9905, not current master: there, a save goes to the sanitized key. Reads and deletes are not rewritten in either. The description, the code comment and the help text now say so, and this PR should merge after the LEAN one. There is no separate issue; the LEAN PR is the driver.
• Read and delete commands (1): get, delete, list and properties send the key as typed again. test_get_sends_unsupported_keys_as_typed, test_delete_sends_an_unsupported_key_as_typed, test_list_sends_an_unsupported_root_key_as_typed and test_properties_sends_an_unsupported_key_as_typed assert the API receives the typed key with no warning and no extra request.
• set overwrites (2): set still sanitizes, like LEAN's save, and now checks whether the sanitized name exists (one properties request, only for unsupported keys) and warns before overwriting it. test_set_warns_when_the_sanitized_key_overwrites_an_existing_object and test_key_to_save_warns_when_the_sanitized_key_overwrites_an_existing_object cover it.
• Testing (4): the end-to-end run against a cloud object store is in the description, including model.v2.pkl stored as typed next to model_v2.pkl and then inspected, downloaded and deleted by its typed name with no warning.
• Fallback issue (minor): keys starting with more than one . reached it; they now get "it starts with more than one '.'", matching LEAN. The fallback stays as a guard.

One observation from the run, unrelated to this change: the object/list endpoint lagged behind sets and deletes by well over 15 seconds, listing a deleted object and omitting new ones.

[quantconnect-kronos-reviewer]

Requesting changes. The rework fixes what I raised last time, and the code is close. What blocks it now is that it depends on QuantConnect/Lean#9905, which is still open with changes requested, and one statement about LEAN's reads doesn't match that PR.

Since my last review

• Fixed: get, delete, list and properties send the key as typed again. The new command tests assert the typed key reaches the API with no extra request.
• Fixed: set warns when the sanitized key already exists. That matches Lean#9905's KeyToSave, which also overwrites and warns.
• Fixed: the premise. The description and the code comment now cite Lean#9905's save behavior. I compared object_store_helper.py with Lean#9905's ObjectStore.cs (SupportedKeyRegex, UnsupportedKeyCharactersRegex, KeyExtensionRegex, FindKeyIssues, SanitizeKey). The regexes, issue wording, dot handling and rules text all match.
• Fixed: the end-to-end run is in the description, and the fallback now has the .. case.

1. Merge order (blocking)
On LEAN master, ReadBytes/ContainsKey don't check the key format. So today lean cloud object-store set "report (1).csv" followed by ObjectStore.Read("report (1).csv") in an algorithm works. If this PR merges while the engine still runs current LEAN, the CLI stores report _1_.csv and that same read finds nothing. Lean#9905 is also still under review, so the rules this PR copies may change. Please hold this PR until Lean#9905 has merged in its final form, then check the helper against it again. I'll approve then.

2. Reads don't match Lean#9905 (description and design)
The description says the read commands send the key as typed, "like LEAN's reads and deletes". Lean#9905 doesn't do that. Its KeyToRead uses the typed key when the store has it, and otherwise falls back to the sanitized key, for ContainsKey, ReadBytes, Delete and GetFilePath. So after set "report (1).csv", an algorithm can read it back as report (1).csv, but lean cloud object-store get "report (1).csv" and properties "report (1).csv" return "File not found". The set warning does name the key to use from then on, so nothing is lost. Please either:

• correct the description, code comment and help text so they don't claim parity with LEAN's reads, or
• for get/properties, mirror KeyToRead: try the typed key first, and fall back to the sanitized key only on "not found". The typed key always wins, so the wrong-object problem from my last review can't come back.

I'd keep delete as typed whichever you choose. A fallback there deletes an object the user didn't name.

Minor

• ObjectStoreClient.exists decides "missing" by looking for "not found" in the error text, as data_downloader.py:94 already does. Any other failure of that properties call now aborts a set of an unsupported key that used to go through. That's acceptable, but a test pinning the exact api message (File not found <key>) would catch a wording change.

Verification: I ran the 47 new and changed object-store tests locally and they pass. set printed the warning and stored the sanitized key, as described.

CI: 34 checks pass and 3 are skipped. One duplicate windows-latest, 3.14 job was still pending when I submitted; the same job passed in the other run.

Other repositories: QuantConnect/Lean#9905 must merge first (above). The api accepts any key and needs no change.

Risk: I kept risk:high. This changes what a user-facing command does with keys users already use, and it depends on an unmerged engine change.

[quantconnect-kronos-reviewer]

Lean#9905 mirrors more than the save. Its KeyToRead falls back to the sanitized key when the typed key isn't stored. The read commands here don't, so get "report (1).csv" after a CLI set returns not found, while an algorithm finds the object. Either mirror that for get/properties, or say here and in the description that only the save is mirrored.

[jhonabreul]

Thanks, addressed in the latest commit.

• Merge order: agreed, this merges only after Improve ObjectStore ergonomics for end-of-run persistence Lean#9905 lands in its final form, and the helper gets checked against it again then. The order across the PRs will be taken care of in time.
• Reads: the description was wrong, the reads stay as typed by design. The description, the helper comment and the set help (README regenerated) now say that only the save mirrors LEAN and that the other commands take the key as typed; the set warning names the key to use. get takes several keys in one request, so a per-key fallback would not fit it, and delete stays as typed, as you suggest.
• exists message: test_exists_returns_false_on_the_api_not_found_message now pins the api's exact message, File not found <key>, as object/properties returns it today for a missing key.

Full unit suite and static_analysis.py pass.

[jhonabreul]

The README check was failing on master as well: the modules list gained the Interactive Brokers (FIX) brokerage after the last run, so scripts/readme.py now adds its option lines. Regenerated in the latest commit, nothing else changed.

[jhonabreul]

The modules list CI downloads has gone back to the one without the Interactive Brokers (FIX) brokerage, twice in a row, so the README check now failed the other way. Reverted the regeneration: the README matches master again, as before this PR's last commit.

[quantconnect-kronos-reviewer]

Requesting changes. Your last commits fixed what I raised on a039b20. Since then, QuantConnect/Lean#9905 has changed how a save handles a sanitized key that already exists, and this PR no longer matches it.

Since my last review

• Fixed: the description, the helper comment and the set help now say that only set mirrors LEAN and the other commands take the key as typed. Keeping get as typed because it takes several keys per request makes sense.
• Fixed: test_exists_returns_false_on_the_api_not_found_message pins the api's exact File not found <key> message.
• The README regeneration and its revert leave the README the same as on master apart from the set help.

1. set overwrites where LEAN now rejects (blocking)
Lean#9905's latest commit (003a8559, "Reject a save whose sanitized key already holds an object written from another key") replaced the overwrite-and-warn behavior that this PR copied. TryGetKeyToSave now returns false, warns, and saves nothing when the sanitized name already holds an object that this store instance didn't write from the same key. A lean cloud object-store set call has no record of earlier saves, so it is in the same position as a fresh LEAN store: when the sanitized key exists, LEAN rejects the save. This PR overwrites it instead (object_store_helper.py:44). For example, set "report (1).csv" replaces an existing report _1_.csv that an algorithm saved directly. LEAN no longer does this, and it is the overwrite I raised in my first review. Please:

• reject in that case: raise an error before any set request, so nothing is written and the command exits non-zero. Use wording close to LEAN's: the sanitized name already holds another object, so delete it or set it directly.
• update the set help ("also when that overwrites an existing object") and regenerate the README.
• change test_set_warns_when_the_sanitized_key_overwrites_an_existing_object and test_key_to_save_warns_when_the_sanitized_key_overwrites_an_existing_object to assert the rejection and that no set request is made.

2. Merge order (still blocking)
Lean#9905 is still open with changes requested, and its save rules changed twice more today (07bb4f5c, 003a8559). As agreed, this PR waits until Lean#9905 merges in its final form. The helper then needs another check against that version. The regexes, issue wording and SanitizeKey dot handling still match 003a8559.

Verification: I ran the 47 object-store tests locally on 71a9c98 and they pass.

CI: 19 jobs show as failed. One is a real failure: test (ubuntu-22.04, 3.13) fails at Check README. The other 18 were cancelled by fail-fast. This PR didn't cause it. The diff only adds the downloaded modules list's Interactive Brokers (FIX) brokerage and its --ib-fix-* options, and master fails the same way at 9ba4213 (test (ubuntu-24.04, 3.13) in that run). Once the modules list settles, someone needs to regenerate the README on master.

Other repositories: QuantConnect/Lean#9905 must merge first. The api accepts any key and needs no change.

Risk: I kept risk:high. This changes where a user-facing command writes, it depends on an unmerged engine change, and it can still overwrite another object (1).

[quantconnect-kronos-reviewer]

"also when that overwrites an existing object" needs updating with the rejection (and the README regenerated).

[jhonabreul]

Thanks, both points addressed in the latest commit.

• set rejects an occupied sanitized key, as LEAN does now. Improve ObjectStore ergonomics for end-of-run persistence Lean#9905 settled on throwing in that case (its latest commit): nothing records whether the object under the sanitized key came from a previous run, another algorithm, an upload or the CLI, so it is only replaced through its exact key. key_to_save now raises before any set request: Key 'report (1).csv' is not supported and its sanitized name 'report _1_.csv' already holds another object. Delete 'report _1_.csv' or set it directly to replace it. The command exits non-zero and nothing is written. set help and README updated; test_key_to_save_raises_when_the_sanitized_key_already_holds_an_object and test_set_fails_when_the_sanitized_key_already_holds_an_object assert the error and that no set request is made. The regexes, issue wording and dot handling are unchanged and still match the Lean PR.
• Merge order is not an issue and should not block the review. The order will be taken care of in time: this PR is merged only after Improve ObjectStore ergonomics for end-of-run persistence Lean#9905, and the helper is checked against the merged version then. Please review the change itself on that basis.
• The README check keeps flapping with the CDN modules list (one run sees the Interactive Brokers (FIX) brokerage, the next does not, master fails the same way), so the README stays as master plus the set help; whoever regenerates it on master once the list settles will carry that hunk.

End to end against a cloud object store under an unused prefix: set "report (1).csv" stored report _1_.csv with the warning; the same set again exited 1 with the rejection above and wrote nothing; set "report _1_.csv" replaced it directly; the object was deleted and confirmed gone.

Full unit suite and static_analysis.py pass.

[jhonabreul]

The LEAN side was split in two: the key rules this PR mirrors (sanitizing unsupported keys, set failing when the sanitized key already holds an object) now live in QuantConnect/Lean#10029, branched off master. QuantConnect/Lean#9905 keeps only the Python helpers. The description now points to #10029; the rules, regexes and messages are unchanged. Merge order is not an issue and should not block the review: it will be taken care of in time, this PR merges after QuantConnect/Lean#10029.