[bugfix] Candidate A for #6690 — reset predicate contexts when a cached index result is reused

[joewiz]

[This PR was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]

This is one of two candidate fixes for #6690. See #6700 for the design question, the trade-offs, and a link to the alternative candidate. Please do not merge this without reading that issue — the two PRs are deliberately competing, not stacked.

Addresses #6690

Summary

An axis step with a name test returned the correct result on its first evaluation and an empty sequence on every evaluation after that, within a single query, whenever the context node carried a predicate context item. Silent — no error, no warning, data loss proportional to loop length.

What Changed

exist-core/src/main/java/org/exist/xquery/LocationStep.java

LocationStep caches its structural-index lookup in currentSet for the whole query execution, and NewArrayNodeSet's sibling and preceding/following select methods stamp those cached NodeProxy objects in place with the matching reference node's context. A duplicate-suppression guard then reads those stamps back — and on the second evaluation it is reading stamps left by the first, so it suppresses every node.

This change clears predicate contexts on currentSet when the cached set is reused rather than freshly built, so the guard only ever sees stamps from the current evaluation. A freshly built set carries no contexts, so only the reuse path needs it.

Applied at the two call sites that feed the guarded select methods: getSiblings and getPrecedingOrFollowing.

Why not simply remove the guard

The guard is still needed. With it disabled the new regression test passes, but ten existing tests fail across axes.xql, npt.xqm and positional-nested.xql — including the file the guard's original 2019 commit added its tests to. It suppresses genuine duplicates between different reference nodes within one evaluation. The bug is the stale state it reads, not the guard.

Known limitation

The previous evaluation's result set holds the same NodeProxy instances as the cached set, so clearing at the start of evaluation n retroactively mutates the result of evaluation n-1. I could not construct a case where that is observable — it is harmless under count() and in every test we have — but it is a real property of this approach and the main reason candidate B exists. This is discussed in #6700.

Test Plan

• New SiblingAxisRepeatedEvaluationRegressionTest fails on unfixed develop, passes with the fix
• Evaluates each affected axis four times over the same node; pins the wildcard form, a node reached without a predicate, an unaffected axis, and a non-matching name alongside
• xquery.CoreTests — the whole src/test/xquery corpus, including the ten tests naive guard removal breaks — green
• Full mvn test on exist-core green
• Codacy PMD clean on the changed file
• license:check clean

Scope

Two call sites in one class. No change to the node-set algorithms or to any hot path.

🤖 Generated with Claude Code

[github-actions]

📊 XQTS result comparison

Comparison of this run against develop.

Warning

70 test cases were recorded in only one of the two runs (0 only in the previous run, 70 only in the current run). The runner's JUnit output is not fully deterministic (see eXist-db/exist-xqts-runner#74), so totals and per-category deltas include recording noise; the newly passing/failing lists count only tests recorded in both runs.

Metric	develop	this run	Change
🟢 Passed	28,780 (90.66%)	28,845 (90.66%)	+65 (+0.00 pp)
🔴 Failures	1,564	1,569	+5
➖ Errors	134	134	0
➖ Skipped	1,267	1,267	0
🧪 Total tests	31,745	31,815	+70

Relative to develop: 0 newly passing, 1 newly failing, 0 new errors, 0 newly skipped — counting only tests recorded in both runs whose outcome changed.

🔴 Newly failing tests (1)

• Constr-inscope-2 (was passing)

⚪ Recorded only in this run (70)

• raytracer (failing)
• fo-test-fn-string-005 (failing)
• fo-test-fn-ends-with-007 (failing)
• fo-test-fn-element-with-id-002 (failing)
• tree-queries-results-q2 (passing)
• functx-fn-year-from-dateTime-1 (passing)
• fo-test-fn-substring-002 (passing)
• fo-test-math-sin-004 (passing)
• fo-test-fn-replace-007 (passing)
• fo-test-math-asin-001 (passing)
• fo-test-math-pow-018 (passing)
• fo-test-fn-tokenize-002 (passing)
• fo-test-math-sin-003 (passing)
• fo-test-fn-days-from-duration-003 (passing)
• fo-test-fn-adjust-time-to-timezone-007 (passing)
• fo-test-fn-concat-002 (passing)
• fo-test-math-sin-009 (passing)
• fo-test-fn-data-004 (passing)
• fo-test-math-log10-005 (passing)
• fo-test-math-tan-008 (passing)
• fo-test-fn-adjust-date-to-timezone-004 (passing)
• fo-test-fn-max-003 (passing)
• fo-test-fn-sort-002 (passing)
• fo-test-math-sqrt-004 (passing)
• fo-test-fn-min-005 (passing)
• fo-test-fn-deep-equal-004 (passing)
• fo-test-fn-starts-with-005 (passing)
• fo-test-fn-matches-006 (passing)
• fo-test-fn-avg-002 (passing)
• fo-test-math-exp-006 (passing)
• fo-test-fn-insert-before-005 (passing)
• fo-test-math-tan-001 (passing)
• fo-test-fn-timezone-from-dateTime-002 (passing)
• fo-test-fn-round-half-to-even-005 (passing)
• fo-test-fn-number-002 (passing)
• fo-test-fn-hours-from-dateTime-003 (passing)
• fo-test-fn-substring-after-002 (passing)
• fo-test-array-get-002 (passing)
• fo-test-math-exp-001 (passing)
• fo-test-fn-not-003 (passing)
• fo-test-math-pow-008 (passing)
• fo-test-fn-abs-002 (passing)
• fo-test-math-atan-007 (passing)
• fo-test-fn-min-003 (passing)
• fo-test-math-pow-034 (passing)
• fo-test-fn-year-from-date-003 (passing)
• fo-test-fn-head-003 (passing)
• fo-test-math-cos-006 (passing)
• fo-test-fn-tokenize-006 (passing)
• fo-test-fn-max-001 (passing)
• … and 20 more

_{Runtime: 369.7s (-39.28s vs develop).}

[joewiz]

[This response was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]

Converting this to a draft. @duncdrum has said on #6700 that candidate B (#6702) looks like the better direction, and I agree — so unless the discussion there turns, this PR is likely to be closed rather than merged.

Leaving it open as a draft for now for two reasons: #6700 is still the live decision and it is easier to compare two things that both exist, and the trade-off recorded in this PR's description and commit message is the clearest statement of what candidate B is buying us. If B lands, I will close this.

Note it now conflicts with develop — #6703 merged and both touch LocationStep. I have deliberately not rebased, since that work is wasted if this is closed. If the decision goes the other way I will rebase and un-draft.