Repository navigation
fix(plugin): bound the url-walk probe with one gtlt range, ready for Harper 5.3; v0.102.1 - #247
harper-joseph wants to merge 1 commit into
Conversation
…Harper 5.3; v0.102.1 Harper 5.3 orders separate search conditions by a storage-level size estimate instead of 5.2's fixed fractions. The walk's end-of-chunk probe asked for `url > cursor` AND `url < endBound` as two conditions; on 5.3 the `< endBound` side can estimate narrower and lead, so finding the one row past the cursor walks every row below the bound. The probe is now one exclusive `gtlt` range, which both 5.2 and 5.3 read as a single bounded scan. Only discovery purges pass an endBound. Also brings the queue keeper's subscription note up to date with 5.3's delivery (harper#2767): a write is delivered only while the row's local entry still carries its version, so the not-owned deletes 5.2 sent are gone. The keeper already ignores not-owned rows and re-checks every entry against its durable row before a claim grants it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the @harperfast/prerender plugin to version 0.102.1, refactoring the URL range walking logic to use a single gtlt comparator instead of separate greater_than and less_than conditions. This change prevents Harper 5.3's query planner from reordering conditions and causing performance issues. The tests and mocks have been updated to support this new comparator. The reviewer pointed out a valid issue in the test mock where the comparator 'greater_than_or_equal' is incorrectly checked instead of 'greater_than_equal', which should be corrected to ensure the mock behaves as expected.
| const gt = conditions.find((c) => c.comparator === 'greater_than')?.value ?? ''; | ||
| const range = conditions.find((c) => c.comparator === 'gtlt')?.value; | ||
| const gt = range?.[0] ?? conditions.find((c) => c.comparator === 'greater_than')?.value ?? ''; | ||
| const ge = conditions.find((c) => c.comparator === 'greater_than_or_equal')?.value; |
There was a problem hiding this comment.
The mock search method looks for the comparator 'greater_than_or_equal', but the actual comparator used by walkUrlRange (in urlWalk.js line 120) is 'greater_than_equal'. This mismatch causes the ge variable to always resolve to undefined during tests, making the mock filter fall back to u > gt (which resolves to u > '' and matches all URLs).
We should correct this to 'greater_than_equal' to ensure the mock filter behaves as intended and remains robust against future test data changes.
| const ge = conditions.find((c) => c.comparator === 'greater_than_or_equal')?.value; | |
| const ge = conditions.find((c) => c.comparator === 'greater_than_equal')?.value; |
Prepares the plugin for Harper 5.3.1. One code change; the rest of the 5.3 behaviour changes were audited and need nothing.
The fix
walkUrlRange's end-of-chunk probe searchedurl > cursorANDurl < endBoundas two conditions. Harper 5.2 ordered them with fixed fractions, so> cursoralways led. Harper 5.3 orders conditions by a storage-level size estimate (harper#2163, #2479). When< endBoundestimates narrower it leads, and finding the single row past the cursor can then walk every row below the bound.The probe is now one exclusive
gtltrange,value: [cursor, endBound]. Both 5.2.14 and 5.3.1 read it as a single bounded scan (search.tssupportsgtltin both). The existing fallback still applies if a store refuses the shape.Only
discoveredPurgepasses anendBound, so this affects discovery purges and nothing on the serve path.renderSchedule.js: the keeper's subscription note now describes 5.3 delivery (harper#2767). A write is delivered only while the row's local entry still carries its version, so the not-owned deletes 5.2 sent are gone. This is a comment-only change: the keeper already ignores not-owned rows and re-checks entries before granting.Audited, no change needed (5.3.0 + 5.3.1)
transaction()(REST) and the operations API bind a request context. The bot path isserver.httpmiddleware with no context, so its writes carry no signal. Forjob_result, a disconnect mid-response means the worker retries the same bytes. Long admin work already runs throughrunDetached.omitCurrent: true.allowFullScan/maxSortRowsnow enforcedTable.searchrefusals are unchanged between versions.searchByIndex, #2259 counters, #2097 array PUT, #2902localOnlygetSabstays harmless.Verification
node --testinpackages/plugin: 2021/2021.npm run lintandnpm run format:checkare clean. The new regression test pins the probe to a singlegtltcondition.harperfast/harper-pro:5.2.14and:5.3.1images. The rehearsal used a production-shaped component config on an--internalDocker network with a fake origin. A dry-run discovery purge over a prefix with neighbouring rows on both sides examined exactly the 300 in-range rows on each version, with 0 errors.job_resultall worked. Records and blobs replicated in both directions, and a peer upgraded mid-run caught up on writes made while it was down. Both logs had 0 errors.Reserves prerender-v0.102.1 (patch; the planned request-sampling feature keeps 0.103.0).
🤖 Generated with Claude Code