Skip to content

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
copilot/refactor-bulk-batch-execution
Feb 25, 2026
Merged

refactor(ktl-api): unify bulk batch execution, parallel fetch, keyword cache, ergonomic cleanup#590
CSWinnall merged 4 commits intofix/ktl-api-issues-raised-by-claudefrom
copilot/refactor-bulk-batch-execution

Conversation

Copy link
Contributor

Copilot AI commented Feb 24, 2026

Addresses five categories of issues raised in the PR #587 review plus targeted performance improvements to the keyword processing pipeline.

API correctness & ergonomics

  • opts.refreshViews accepted on all six write methods (createRecord, updateRecord, deleteRecord, createRecords, updateRecords, deleteRecords) — eliminates null/[] positional placeholders
  • staggerMs * index → flat staggerMs in _runBatchWorkers — old formula stacked N×delay on top of real processing time with bounded workers
  • debugger statements removed from bulk-edit null-check guards
  • _buildRequestError hardenedresponseText extraction guards against non-string values
  • Double error handling in _request eliminated — inner catch now early-throws structured errors without a redundant isRetryable re-check
  • createRecordWithAsset removed — fully superseded by createRecord/updateRecord with autoUploadAssets: true; _prepareRecordDataForCreate_prepareRecordData and wired into both

New API surface

  • findRecords(viewId, fieldId, value, options) — equality-filter shorthand delegating to getAllRecords
  • updateRecords per-record shape — accepts Array<{id, data}> so each record gets different field values; old shared-data shape unchanged
await ktl.api.updateRecords(viewId, [
    { id: 'abc123', data: { field_1: 'Alice' } },
    { id: 'def456', data: { field_1: 'Bob' } }
]);
  • ktl.api exposed on the public KTL object

Performance

  • Parallel page fetching in getAllRecords / getAllChildRecords — pages after the first are fetched in concurrent batches (default 5, tunable via options.pageConcurrency); a 50-page dataset drops from ~50 serial round-trips to ~10 parallel rounds
  • In-flight GET deduplication_request split into wrapper + _requestInner; identical concurrent GET URLs share one fetch via _inflightGets: Map, avoiding redundant hits when multiple components request the same view simultaneously
  • getFieldKeywords fast path — field keywords are pre-parsed into ktlKeywords at startup; function now does a direct Map lookup instead of re-running regex on raw descriptions every call
  • getAllFieldsWithKeywordsInView per-session cache (_fieldsWithKwCache: Map) in ktl.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 session

Error codes

  • KEC_1027 split into KEC_1028 (create), KEC_1029 (update), KEC_1030 (delete), KEC_1031 (tags)
  • New KEC_1032 for bulk-edit source-row/record-not-found guard
Original 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

  1. Breaking rename with no backward-compat shim
    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.

  1. debugger statements in production code (2 places)
    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.

  1. _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.

  2. 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.

  3. 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.

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
Copilot AI requested a review from CSWinnall February 24, 2026 21:49
…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 CSWinnall marked this pull request as ready for review February 25, 2026 07:57
@CSWinnall CSWinnall merged commit fdcca22 into fix/ktl-api-issues-raised-by-claude Feb 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants