refactor(ktl-api): unify bulk batch execution, parallel fetch, keyword cache, ergonomic cleanup#590
Merged
CSWinnall merged 4 commits intofix/ktl-api-issues-raised-by-claudefrom Feb 25, 2026
Conversation
Co-authored-by: CSWinnall <10554243+CSWinnall@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor bulk batch execution and standardize progress and logging
fix(ktl-api): resolve five post-review issues from PR #587 refactor
Feb 24, 2026
…Records, ergonomic cleanup Co-authored-by: CSWinnall <10554243+CSWinnall@users.noreply.github.com>
Copilot
AI
changed the title
fix(ktl-api): resolve five post-review issues from PR #587 refactor
feat(ktl-api): parallel page fetching, per-record bulk updates, findRecords, ergonomic cleanup
Feb 24, 2026
… all batch methods Co-authored-by: CSWinnall <10554243+CSWinnall@users.noreply.github.com>
Copilot
AI
changed the title
feat(ktl-api): parallel page fetching, per-record bulk updates, findRecords, ergonomic cleanup
refactor(ktl-api): unify bulk batch execution, parallel fetch, keyword cache, ergonomic cleanup
Feb 24, 2026
CSWinnall
approved these changes
Feb 25, 2026
fdcca22
into
fix/ktl-api-issues-raised-by-claude
1 check passed
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 free
to 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.
Addresses five categories of issues raised in the PR #587 review plus targeted performance improvements to the keyword processing pipeline.
API correctness & ergonomics
opts.refreshViewsaccepted on all six write methods (createRecord,updateRecord,deleteRecord,createRecords,updateRecords,deleteRecords) — eliminatesnull/[]positional placeholdersstaggerMs * index→ flatstaggerMsin_runBatchWorkers— old formula stacked N×delay on top of real processing time with bounded workersdebuggerstatements removed from bulk-edit null-check guards_buildRequestErrorhardened —responseTextextraction guards against non-string values_requesteliminated — inner catch now early-throws structured errors without a redundantisRetryablere-checkcreateRecordWithAssetremoved — fully superseded bycreateRecord/updateRecordwithautoUploadAssets: true;_prepareRecordDataForCreate→_prepareRecordDataand wired into bothNew API surface
findRecords(viewId, fieldId, value, options)— equality-filter shorthand delegating togetAllRecordsupdateRecordsper-record shape — acceptsArray<{id, data}>so each record gets different field values; old shared-data shape unchangedktl.apiexposed on the public KTL objectPerformance
getAllRecords/getAllChildRecords— pages after the first are fetched in concurrent batches (default 5, tunable viaoptions.pageConcurrency); a 50-page dataset drops from ~50 serial round-trips to ~10 parallel rounds_requestsplit into wrapper +_requestInner; identical concurrent GET URLs share one fetch via_inflightGets: Map, avoiding redundant hits when multiple components request the same view simultaneouslygetFieldKeywordsfast path — field keywords are pre-parsed intoktlKeywordsat startup; function now does a direct Map lookup instead of re-running regex on raw descriptions every callgetAllFieldsWithKeywordsInViewper-session cache (_fieldsWithKwCache: Map) inktl.views— function was called 15+ times per view render cycle; view structure and keyword assignments are immutable after startup so the cache is safe for the full sessionError codes
KEC_1027split intoKEC_1028(create),KEC_1029(update),KEC_1030(delete),KEC_1031(tags)KEC_1032for bulk-edit source-row/record-not-found guardOriginal prompt
PR #587 — refactor(ktl-api): unify bulk batch execution + standardize progress and logging
What it does
$.ajax → fetch — Core _request transport migrated from jQuery Ajax to native fetch with AbortController timeouts
Bulk ops refactored — createRecords, updateRecords, deleteRecords now use bounded worker concurrency (_runBatchWorkers) instead of unbounded Promise.all/Promise.allSettled
API renames — getRecordChildren → getChildRecords, getAllRecordChildren → getAllChildRecords, param connectionFieldKey → connectionSlug
New features — uploadAsset(), createRecordWithAsset(), auto-upload via autoUploadAssets option on createRecord
Error code split — Old shared KEC_1027 split into KEC_1028 (create), KEC_1029 (update), KEC_1030 (delete), KEC_1031 (tags)
UX — Bulk progress now counts up ("X of total") instead of counting down
Bulk edit safety — Null checks for missing source rows/records with user-facing error popups
ktl.api exposed — Added api: this.api to the public KTL object
Issues I see
getRecordChildren / getAllRecordChildren are renamed to getChildRecords / getAllChildRecords. Any external app code calling the old names will silently fail. I didn't find callers in your repos, but Craig or other KTL users might have code using them. Consider adding deprecated aliases:
getRecordChildren: function (...args) { return apiInstance.getChildRecords(...args); },
2. Stagger delay scales with absolute index — will be very slow on large batches
In _runBatchWorkers, the delay is staggerMs * index where index is the absolute item number (0–99, etc.). With bounded workers, by the time a worker picks up item 60, it has already waited for ~10 previous items to complete. Then it adds 60 * 40 = 2400ms of stagger on top. Item 99 would add ~4 seconds of pure waiting. The old unbounded code had the same formula but all delays ran concurrently from time zero. With bounded workers, this delay accumulates on top of actual processing time, making large batches significantly slower. The fix would be a flat await delay(staggerMs) per iteration instead of staggerMs * index.
At the new null-check guards for bulk edit source rows (~line 31358, 31373):
if (typeof ktl?.account?.isDeveloper === 'function' && ktl.account.isDeveloper()) {
debugger;
}
Even gated behind isDeveloper(), this will freeze the browser tab if DevTools is open. Probably fine for debugging but should be removed before release.
_buildRequestError receives fetch-shaped objects but reads .responseText
The current _buildRequestError at line 5288 reads jqXHR?.responseText. The new fetch callers pass { status, statusText, responseText, headers } — this works because the property name matches. But if an actual fetch Response object ever gets passed directly (it has .text() method, not .responseText property), it would silently produce body: null. The current call sites are correct, but it's fragile.
KEC_1027 still exists on dev in tags code (lines 14253, 14349)
The PR changes these to KEC_1031, but the current dev branch still has 5 places using KEC_1027. The PR correctly splits all of them — just verify the merge won't leave orphaned KEC_1027 references if there have been other changes to those lines.
Minor: Double error handling in _request
When a non-retryable HTTP error occurs, it's thrown inside the try block, caught by the outer catch, re-checked, and re-thrown. Functionally correct but unnecessarily redundant — the inner throw and outer catch do the same retryable check twice.
What to test
High priority (core transport change):
Any page that loads data via KTL API views — confirm records still load
Single record create, update, delete — verify basic CRUD still works
Trigger a bulk edit operation (paste multiple values) — verify progress shows "X of Y" counting up
Trigger a bulk copy operation — verify progress shows "X of Y"
Test with a bad/expired session to verify error handling (should get clean error, not crash)
If you can trigger a 429 rate limit, verify retry/backoff still works
Bulk operations:
Bulk update ~20+ records — watch progress and verify all records update
Bulk delete — same thing
Bulk create (copy) — verify records are created and progress is accurate
Test with continueOnError: false (default) — one failure should stop the batch
Click a bulk edit/copy button before the view fully loads — should get the new "source row not found" popup instead of crashing
New features (if you use them):
ktl.api.uploadAsset(file) — upload a file and verify it returns an asset ID
ktl.api.createRecordWithAsset(viewId, fieldId, file, data) — create a record with a file attachment
Renames (if any external code uses the old names):
Search your Knack app customization code for getRecordChildren or getAllRecordChildren
Bottom line: The refactor is solid architecturally — bounded concurrency is a real improvement over unbounded Promise.all. The $.ajax ...
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.