fix(oas:sync): reuse an existing tag folder regardless of hyphen/space spelling - #51
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
…e spelling Tag-derived category folders were only lowercased, not slugified, so a spec tag with spaces (e.g. "Shipping Labels") produced a folder named literally "shipping labels" instead of landing on the platform's own hyphenated "shipping-labels" folder — creating a duplicate category on every sync instead of reusing the existing one. Hyphen vs. space isn't a "one spelling is correct" situation: they're just two spellings of the same folder. Added `slugifyFolder()`, used both as an equivalence key (so "Shipping Labels" / "shipping labels" / "shipping-labels" are recognized as the same folder) and, when nothing exists under any spelling yet, as the spelling used to create a new folder, matching what a fresh platform OAS-upload would produce. Added `resolveFolder()`, memoized per OAS file per sync run, which looks for an existing folder whose slugified name matches before falling back to the hyphenated default — used both when placing new operation pages and when backfilling a missing category index.md, so whatever's already on disk wins. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
74074fb to
89dcfd2
Compare
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
rossrdme
changed the title
|
|
||
| let resolved = slug; | ||
| try { | ||
| for (const entry of fs.readdirSync(apiDir, { withFileTypes: true })) { |
There was a problem hiding this comment.
how many entries could we see here? hundreds? Thousands? 10k+? I worry about the perf of walking each item like this.
There was a problem hiding this comment.
(Response from Claude) Replaced the per-slug directory walk with a single readdirSync call into a slug -> actual name map. Folder resolution is now an O(1) lookup — O(entries) total per API file instead of O(tags × entries).
| // to be created). Memoized per OAS file per run. | ||
| const apiDir = path.join(refDir, infoTitle); | ||
| const resolvedFolders = new Map(); | ||
| function resolveFolder(slug) { |
There was a problem hiding this comment.
🔧 if we can. lets not declare this inside of a function.
There was a problem hiding this comment.
(Response from Claude) Moved to a top-level existingFoldersBySlug() helper, matching the other helpers in this file. Also resolves the performance issue noted below.
…a nested closure Addresses review feedback from flinehan on #51: - resolveFolder() re-walked apiDir with fs.readdirSync on every distinct tag/group not yet memoized, which is O(tags x entries) for an API with many tags and a large existing reference tree. - resolveFolder() was declared as a closure nested inside syncOneOas, inconsistent with every other helper in this file being top-level. Replaces it with existingFoldersBySlug(apiDir), a top-level function that builds a slug -> actual-name Map with a single readdirSync call; folder resolution at each call site is now foldersBySlug.get(folder) || folder, an O(1) lookup. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
2 participants
Summary
safeSegment(op.tag).toLowerCase()), not slugified. A spec tag containing spaces (e.g.Shipping Labels) produced a folder named literallyshipping labels, instead of landing on the platform'sshipping-labelsfolder already on disk — so every sync created a duplicate category folder for the same tag instead of reusing the existing one.oas:synccreated brand-new space-separated duplicate folders alongside the hyphenated ones the platform's own OAS-upload had already created on disk.slugifyFolder(), applied inoperationGroup()(both the tag-folder and untagged path-folder branches) and in thedeclaredOrdertag-ordering computation insyncOneOas. It's used both as an equivalence key (to recognize e.g.Shipping Labels/shipping labels/shipping-labelsas the same folder) and, when nothing already exists under any spelling, as the spelling used to create a new folder — matching what a fresh platform OAS-upload would produce.resolveFolder(), memoized per OAS file per sync run, looks for an existing folder whose slugified name matches before falling back to creating the hyphenated default. Used both when placing new operation pages and when backfilling a missing categoryindex.md, so an operation always lands in whichever folder is actually there instead of spawning a second, differently-spelled one next to it.Test plan
node --test src/**/*.test.js test/*.test.js— 184/184 passing