Skip to content

Fix findRecords filter merge to preserve caller boolean logic.#592

Open
CSWinnall wants to merge 7 commits intodevfrom
fix/ktl-api-find-records-and-minor-updates
Open

Fix findRecords filter merge to preserve caller boolean logic.#592
CSWinnall wants to merge 7 commits intodevfrom
fix/ktl-api-find-records-and-minor-updates

Conversation

@CSWinnall
Copy link
Collaborator

Previous behavior flattened caller rules into an and group when caller passed match/rules, which could silently convert caller or intent into and. New behavior nests caller filters as a single sub-rule under the base equality filter, preserving caller match semantics. Improve paged read resilience in getAllRecords and getAllChildRecords.

Replace Promise.all batch fetch with allSettled handling so successful pages are retained even if one page fails. Add optional failOnPageError to support strict fail-fast behavior when required. Add warning logs with page context for failed pages. Route uploadAsset through the shared request pipeline.

Remove standalone fetch path and use common request logic for retries, backoff, timeout, and 429 handling consistency. Maintain write queue integration and existing return contract. Add configurable asset-size policy in uploadAsset.

New create-level and per-call controls: warnAssetSizeBytes maxAssetSizeBytes enforceAssetSize Warn when size exceeds warning threshold. Reject early with clear error when enforcement is enabled and max size is exceeded. Add human-readable byte formatting helper for diagnostics. Simplify request error construction.

Remove dead response-like shape branch that was never hit by current call paths. Keep structured error contract with status and body. Harden updateRecords input-shape detection.

Validate all elements before entering per-record mode. Throw clear error for mixed arrays instead of failing later during execution. Documentation updates.

Expand KTL API docs with new options, defaults, behavior changes, and read-path error semantics. Add AI workflow rule to check existing docs first, update when present, and only create docs for complex features.

CSWinnall and others added 5 commits February 27, 2026 08:12
Previous behavior flattened caller rules into an and group when caller passed match/rules, which could silently convert caller or intent into and.
New behavior nests caller filters as a single sub-rule under the base equality filter, preserving caller match semantics.
Improve paged read resilience in getAllRecords and getAllChildRecords.

Replace Promise.all batch fetch with allSettled handling so successful pages are retained even if one page fails.
Add optional failOnPageError to support strict fail-fast behavior when required.
Add warning logs with page context for failed pages.
Route uploadAsset through the shared request pipeline.

Remove standalone fetch path and use common request logic for retries, backoff, timeout, and 429 handling consistency.
Maintain write queue integration and existing return contract.
Add configurable asset-size policy in uploadAsset.

New create-level and per-call controls:
warnAssetSizeBytes
maxAssetSizeBytes
enforceAssetSize
Warn when size exceeds warning threshold.
Reject early with clear error when enforcement is enabled and max size is exceeded.
Add human-readable byte formatting helper for diagnostics.
Simplify request error construction.

Remove dead response-like shape branch that was never hit by current call paths.
Keep structured error contract with status and body.
Harden updateRecords input-shape detection.

Validate all elements before entering per-record mode.
Throw clear error for mixed arrays instead of failing later during execution.
Documentation updates.

Expand KTL API docs with new options, defaults, behavior changes, and read-path error semantics.
Add AI workflow rule to check existing docs first, update when present, and only create docs for complex features.
Co-authored-by: CSWinnall <10554243+CSWinnall@users.noreply.github.com>
Honor `options.refreshViews` in `updateRecords` per-record invocation
Prevents use of nested filter groups in filter parameters by explicitly rejecting them with clear error messages, due to Knack API limitations. Also disallows merging filters with top-level 'or' logic to avoid unreliable behavior, instructing users to run separate queries and merge results client-side. Improves documentation to clarify these restrictions and error handling.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the KTL API module's reliability and consistency by improving filter handling, adding resilience to paged reads, standardizing the upload pipeline, and hardening input validation.

Changes:

  • Validates filter groups in findRecords to prevent silent OR-to-AND conversion, rejecting unsupported nested groups and non-AND logic with clear errors
  • Replaces Promise.all with Promise.allSettled in paged reads to retain successful pages when batch members fail, with optional failOnPageError for strict behavior
  • Routes uploadAsset through the shared _request pipeline for consistent retry/backoff/timeout handling, adding configurable size policy controls (warn threshold, max size, enforcement toggle)
  • Hardens updateRecords to validate input shape upfront and throw clear errors for mixed arrays instead of failing during execution
  • Adds documentation maintenance rules and updates API docs to reflect new options, defaults, and error semantics

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
KTL_AI_Instructions.md Adds documentation maintenance workflow rules
KTL.js Implements filter validation, Promise.allSettled resilience, unified upload pipeline, asset size policy, input validation hardening, and helper utilities
Docs/KTL_API.md Documents new options, defaults, behavior changes, and error handling semantics

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

_formatByteSize(bytes) {
if (!Number.isFinite(bytes) || bytes < 0) return '0 B';
const units = ['B', 'KB', 'MB', 'GB', 'TB'];
if (bytes === 0) return '0 B';
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check on line 5893 already returns '0 B' when bytes is 0 or negative, making the explicit if (bytes === 0) check on line 5895 unreachable. This line can be removed as it's redundant.

Copilot uses AI. Check for mistakes.
KTL.js Outdated
Comment on lines 4786 to 4822
const fileSizeBytes = Number.isFinite(file?.size) ? file.size : 0;
const warnAssetSizeBytes = Number.isFinite(opts.warnAssetSizeBytes) && opts.warnAssetSizeBytes >= 0
? opts.warnAssetSizeBytes
: this.options.warnAssetSizeBytes;
const maxAssetSizeBytes = (opts.maxAssetSizeBytes === null)
? null
: (Number.isFinite(opts.maxAssetSizeBytes) && opts.maxAssetSizeBytes > 0
? opts.maxAssetSizeBytes
: this.options.maxAssetSizeBytes);
const enforceAssetSize = opts.enforceAssetSize === true
? true
: (opts.enforceAssetSize === false ? false : this.options.enforceAssetSize);

if (Number.isFinite(warnAssetSizeBytes) && warnAssetSizeBytes >= 0 && fileSizeBytes > warnAssetSizeBytes) {
this._log('Asset upload exceeds warning threshold', {
fileName: typeof file?.name === 'string' ? file.name : 'upload.bin',
fileSizeBytes,
warnAssetSizeBytes,
fileSizeLabel: this._formatByteSize(fileSizeBytes),
warnSizeLabel: this._formatByteSize(warnAssetSizeBytes)
}, 'warn');
}

if (Number.isFinite(maxAssetSizeBytes) && maxAssetSizeBytes > 0 && fileSizeBytes > maxAssetSizeBytes) {
const sizeMessage = `KTL API error: uploadAsset file size ${this._formatByteSize(fileSizeBytes)} exceeds configured max ${this._formatByteSize(maxAssetSizeBytes)}.`;
if (enforceAssetSize) {
throw new Error(sizeMessage);
}

this._log('Asset upload exceeds configured max size but enforcement is disabled', {
fileName: typeof file?.name === 'string' ? file.name : 'upload.bin',
fileSizeBytes,
maxAssetSizeBytes,
fileSizeLabel: this._formatByteSize(fileSizeBytes),
maxSizeLabel: this._formatByteSize(maxAssetSizeBytes)
}, 'warn');
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When file.size is not finite, fileSizeBytes defaults to 0 (line 4786), which bypasses all size checks since 0 is never greater than any threshold. This means files without valid size information will silently skip warning and enforcement checks. Consider whether files with unknown size should be: (1) allowed without size checks (current behavior), (2) rejected when enforcement is enabled, or (3) at minimum logged as a warning. This could be relevant for edge cases where the File object doesn't properly expose its size property.

Copilot uses AI. Check for mistakes.
Improves asset upload logic by detecting unknown file sizes and bypassing size warning/enforcement for such files, logging a warning instead.

Prevents false positives and errors when file size metadata is unavailable.
Refines file size handling for asset uploads, ensuring enforcement
is robust when file size is unknown and clarifying warning/validation
flows. Adds detailed documentation for exposed API methods to
enhance developer clarity and usability.
@CSWinnall CSWinnall requested a review from cortexrd February 27, 2026 17:04
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.

3 participants