Uh oh!
There was an error while loading. Please reload this page.
fix(data ingest): mis-cased media folder next to labels.csv stays ambiguous, not confident tabular (#203) - #211
Merged
Conversation
…a folder as a table (#203) A dataset with labels.csv plus a mis-cased media folder (Images/, Texts/, Sequences/) already stays ambiguous rather than sniffing confident tabular (#198's miscasedMarker guard) — but the ambiguous path gave no clue why. So the user saw only a blind "which is it?" prompt with no hint that their folder was just mis-cased. Carry a plain-language hint on the ambiguous FamilySniff for that case, naming the folder and the lowercase form it should have (e.g. "Images" → "images"), and surface it via ui.Printer.Warnf in resolveFamily before the family question. Reuses the same marker set the case-sensitive walk keys on (markerFold, formerly isMarkerFold, now returns the canonical marker). Extend TestSniffFamily to cover all three mis-cased markers (adds the Sequences/ case), assert each stays ambiguous AND carries a hint naming the rename, and pin that an unrelated subdir stays confident tabular with no hint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#203) Address the code-review findings on the mis-cased-media guard. - Cross-platform false positive: the guard flagged a mis-cased "Images/" next to labels.csv as ambiguous and told the user to rename it — but on a case-insensitive filesystem (macOS APFS, Windows) the walk's own os.Lstat(<dir>/images) resolves that folder, so Discover already accepts the layout. The CLI preflight runs on the user's own machine, so a valid layout was reported broken with an unnecessary rename instruction. Now the sniff probes the literal lowercase marker path the walk keys on (markerResolves): if it resolves, treat the folder as the real marker (confident media, no hint); only when it doesn't resolve (case-sensitive FS, e.g. Linux) is it the genuine #203 footgun — stay ambiguous + hint. - Test coverage: add TestResolveFamily_SurfacesMiscasedHint, pinning that resolveFamily surfaces the advisory hint through the printer before asking the family plainly (the PR's headline behavior, previously exercised by no test). FS-aware, so on case-sensitive CI deleting the Warnf branch fails it. - Make the SniffFamily mis-cased test FS-aware to match the new behavior, and drop the dead `&& s.Confident` assertion (unreachable after the preceding not-confident guard). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
self-requested a review
July 10, 2026 10:06
saadqbal
commented
Jul 10, 2026
Collaborator
Reviewed at high effort — clean PR 👍 Checked the one thing that matters here: |
saadqbal
approved these changes
Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a dataset folder holds
labels.csvnext to a mis-cased media folder (e.g.Images/instead ofimages/), the family sniff used to fall through to confident tabular and the guided flow silently ingested the tree as a table — dropping the images/texts. #203.The first pass added a guard that flagged any mis-cased marker folder as ambiguous with a rename hint. Code review surfaced that this over-corrected on case-insensitive filesystems: on macOS (APFS) and Windows the walk's own
os.Lstat(<dir>/images)resolvesImages/, soDiscoveralready accepts the layout. Since the CLI preflight runs on the user's own machine, a user with a perfectly valid layout was told it was broken and instructed to perform an unnecessary rename.Fix
Make the sniff track the walk on the filesystem it runs on instead of guessing from the on-disk casing:
markerResolves→os.Lstat(filepath.Join(dir, "images")), mirroringDiscover/DiscoverText).The heuristic stays narrow and consistent with
Discover*(RFC-0002 §5.1 Principle 6): the sniff never claims more than the matching walk would accept.Acceptance
labels.csv+Images/: sniff is confident image, no rename hint, andDiscoveraccepts the same tree.labels.csv+Images/: sniff is ambiguous, carries a rename hint namingImages→images, and the family question is still asked.backup/) stays confident tabular, no hint.Test plan
go build ./... && go test ./... && gofmt -l . && go vet ./...— all green locally (case-insensitive branch).TestSniffFamilymis-cased cases are now FS-aware: they assert confident media on a case-insensitive FS and ambiguous + rename hint on a case-sensitive one, tied to the walk's own probe. Linux CI exercises the footgun branch.TestResolveFamily_SurfacesMiscasedHintpins thatresolveFamilysurfaces the hint via the printer before asking the family plainly — the PR's headline behavior, previously untested. Deleting theWarnfbranch fails it on case-sensitive CI.&& s.Confidentassertion (unreachable after the preceding not-confident guard).Closes#203
🤖 Generated with Claude Code
Note
Low Risk
Changes are limited to interactive family sniff/preview and CLI messaging; behavior aligns with existing Discover* walks rather than introducing new ingest rules.
Overview
Fixes #203, where
labels.csvbeside a mis-cased media folder (e.g.Images/) could sniff as confident tabular and silently drop media. The sniff now mirrors the real walk viamarkerResolves(os.Lstaton the lowercase marker path): on case-insensitive FS it treats the folder as a real marker (confident image/text, no hint); on case-sensitive FS it stays ambiguous, blocks the tabular masquerade, and setsFamilySniff.Hintwith a rename message.resolveFamilyprints that hint withWarnfbefore the family question (hint stays advisory). Tests are FS-aware for mis-casedImages/Texts/Sequences, plusTestResolveFamily_SurfacesMiscasedHintfor the CLI path.Reviewed by Cursor Bugbot for commit 847ac11. Bugbot is set up for automated code reviews on this repo. Configure here.