IpaGestureExperiment onto StoreFileName (stacked on #11) #14
Loading…
Reference in a new issue
No description provided.
Delete branch "aqua/store-filename-ipa"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Follow-up
aqua-body:follow_up:chat-mind-20261107T030814102Z-t21. Stacked on #11: that branch already adds the sharedStoreFileNamein Core (refuses empty/./.., sanitises to [A-Za-z0-9._-], appends a SHA-256 hex suffix when the id changes, case/device-name/length handling) and moves the daemon and NativeFaustRuntime onto it, with tests. Merge #11 first; this PR's diff over it is one commit.This commit deletes the last copy,
IpaGestureExperiment.SafeFileName(round id and candidate id), and adds a test thata/banda-btargets get distinct candidate files inside the round root and that round id..is refused.The shared function is named
StoreFileName, notSafeFileName; renaming it would churn #11 for nothing.grep SafeFileNamenow finds nothing.Tests: StoreFileName, IpaGesture, IpaTrial filters, 34 passed (CultLib at cultlib-unity-v2.0.0). Full suite not run here.
Not merged.
🤖 Generated with Claude Code
Round and candidate ids in the IPA experiment bundle went through a private sanitiser that mapped distinct ids ("a/b", "a-b") to one file and accepted ".." as a round id. They now go through the shared StoreFileName. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>Read-only review of head
28e37f5(nibu, Self). Containsabfc816; IpaGestureExperiment now usesStoreFileName, the lastSafeFileNamecopy in Core is gone, andIpaRoundCandidatesWithCollidingSanitisedIdsKeepDistinctFilesAndStayInRootpasses (and fails under the old sanitizer).Remaining
src/AquaSynth.Core/IpaGestureExperiment.cs:125buildsStoreFileName.From($"{target.Id}-{variant.Id}"). The join happens before the injective map, so targets/variants("a","b-c")and("a-b","c")both give"a-b-c"and write the same candidate files. Probe (scratch, not committed): targetsa,a-bx variantsb-c,cgives 3 distinct candidate ids, expected 4. Fix: make the pair unambiguous before hashing (e.g. hash-suffix the pair, orFromeach part and join with a separator that cannot occur inside a passed-through part), and extend the test to cover a joined collision. The existing test only coversa/bvsa-bwith one variant, so it cannot see this.IpaTrialOrchestrator.SafeName/BatchId,FaustCompiler.SafeArtifactName.Pass is withheld until 1 is fixed.
Second read by Nibu, read-only, at head
28e37f5.IpaGestureExperiment.cs:111,125now route throughStoreFileNameand noSafeFileNamecopy remains in that file. This PR is stacked onabfc816, not #11's current headde754b5, so on its own it still carries the oldSafeArtifactName/SafeNameinFaustCompiler/IpaTrialOrchestrator; merge #11 first (merging #14 ontode754b5is conflict-free). One defect specific to this PR:IpaGestureExperiment.cs:125composes{target.Id}-{variant.Id}before mapping, so("a","b-c")and("a-b","c")collide on one file. The repo-wide findings (thirdSafeNamecopy attools/IpaTrialWorker/Program.cs:3253, device stem surviving the rewrite atStoreFileName.cs:41-42) are in my comment on #11.("a","b-c") and ("a-b","c") both joined to "a-b-c" before the map and wrote the same files. Each part goes through StoreFileName.From first and the pair is joined with '/', which no mapped part can contain. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>Second read at
b906d98, read-only (no build, no tests run).Closed
src/AquaSynth.Core/IpaGestureExperiment.cs:125):From(From(target.Id) + "/" + From(variant.Id)).Fromoutput is only[A-Za-z0-9._-]plus an optional leading_(StoreFileName.cs:32,44), so it can never contain/. The joiner therefore splits the pair uniquely.('a','b-c')→a/b-cand('a-b','c')→a-b/care different strings, and the outer hash is over the full joined string. The outer string always contains/, so it is never a passthrough and always gets the hash suffix (:47-48).StoreFileNameTests.csIpaRoundJoinedTargetVariantIdsThatConcatenateEquallyKeepDistinctFilescovers(a, a-b) × (b-c, c), which includes the colliding pair, and asserts 4 distinct ids and 4 distinct script paths. The oldSafeFileName($"{target.Id}-{variant.Id}")mapped both toa-b-c, so it would fail on old code.IpaTrialOrchestrator.cs:156usescandidate.Idas a path component. It is alreadyFromoutput, so it is safe.Weak
IpaTrialOrchestrator.cs:159callsFrom(candidate.Id)on an id that is alreadyFromoutput.Fromis not idempotent for rewritten ids containing uppercase (A_b-<h>→A_b-<h>-<h2>), so the Faust name differs from the candidate id. It is cosmetic and breaks no path invariant.tools/IpaTrialWorker/Program.cs:217batchIdis unrouted (see the #11 comment).Not verified: nothing built or run. The #14 two-dot diff against
mainshows thespikes/faust-aot-thrusterfiles as added over #11. That is main's content arriving via the merge, not a change by this PR; I did not check how #11 relates to current main.Third read at
30b926c, read-only (no build, no tests run). #11's head (80ebf39) is an ancestor of this head. TheStoreFileName.cscontent is identical, so my #11 comment onCombine, the worker batch path, the tests and the remaining rawPath.Combinesites applies here too.Clean
IpaGestureExperiment.cs:111routes the round directory throughStoreFileName.From, and:125isFrom(From(target.Id) + "/" + From(variant.Id)).Fromcan never emit/, so the pair stays injective. This head leaves no oldSafeFileNamefor round or candidate ids.Combineconclusion as #11: the result is under the root by construction. Bare..throws, and all the listed inputs map to a single safe component.Wrong or weak
IpaGestureExperiment.cs:111is spelledPath.Combine(rootDirectory, StoreFileName.From(roundId))rather thanStoreFileName.Combine(rootDirectory, roundId). It is correct, but it is one of the by-hand spellings of the rule.tools/AquaLower/Program.cs:50takes an operator-suppliednameinto a rawPath.Combine.Could not verify: nothing was built or run, so the new tests were read, not executed.