fix(engine): ordered walk under an active AOF follows the index, not recno

[Admnwk]

Summary

With an AOF/filter active AND an index order set, navigation walked the
table in recno order instead of the active index order — so a filtered,
ordered browse came out unsorted. This is a frequent migration breaker (any
grid that combines SET ORDER with a filter).

Root cause

install_aof_bitmap builds the M-AOF.5 sparse recno_sequence_ in recno
order, and goto_top / goto_bottom / skip prefer that sequence over the
active index. The bitmap predicate (filter_) was already correct; only the
sparse-walk order was wrong.

Fix

Build the AOF sparse sequence in the active index traversal order (recno
order when no order is set), via a small rebuild_aof_sequence_() helper, and
rebuild it whenever the active order changes (set_order / clear_order).

• The recno_sequence_ consumers (record count, skip stepping) are unchanged —
  only its order is corrected.
• It is a no-op when no AOF is active, so the non-filtered paths and the
  SQL ORDER BY result sequence are untouched.

Discovery / testing

Found via the tools/qa-diff differential harness (rddads vs native DBFCDX:
filter AGE>=30 walked out of order) and confirmed at the ABI (no rddads)
by the new regression test abi_aof_order_walk_test — ordered walk under an
active AOF, plus AOF re-apply and AOF-clear.

Suite: 876/876, /WX (warnings-as-errors) clean, MSVC x64.

🤖 Generated with Claude Code (Opus 4.8)

[gemini-code-assist]

Code Review

This pull request addresses an issue where ordered walks under an active Advantage Optimized Filter (AOF) did not respect the active index order. It introduces rebuild_aof_sequence_ to align the sparse record number sequence with the active index order, and adds a regression test to verify this behavior. The review feedback highlights two important issues: first, the current implementation of rebuild_aof_sequence_ ignores active index scopes, which can bypass scope boundaries and return incorrect records; second, the test code contains a potential buffer overflow when copying the directory path into a fixed-size buffer.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Respect Active Index Scopes in AOF Sequence Rebuild

The current implementation of rebuild_aof_sequence_ walks the entire index from the beginning (seek_first) to the end (seek_last), completely ignoring any active index scopes (order_->scope().top and order_->scope().bottom). This causes any active scope to be bypassed when an AOF is active, resulting in records outside the scope being returned.

To fix this correctness issue:

1. Respect the scope boundaries by starting the traversal at the appropriate scope boundary (e.g., seek_key with the top scope for ascending traversal, or bottom scope for descending traversal).
2. Terminate the traversal early once the keys go past the active scope boundaries.
3. Ensure defensive programming by checking if driver_ is valid before dereferencing it.

Note: Since changing the scope also affects the visible set of records, you should also update set_scope, clear_scope, and clear_scopes to call rebuild_aof_sequence_() so that the cached sequence is correctly rebuilt when scopes change.

void Table::rebuild_aof_sequence_() {
    if (!aof_active_ || !driver_) return;
    std::vector<std::uint32_t> seq;
    if (order_ && order_->index()) {
        // Walk the active index in traversal order, keeping only AOF matches,
        // so the sparse walk follows the index (not recno) order.
        auto* idx = order_->index();
        idx->invalidate_cursor();
        const bool desc = order_->descending_traverse();
        
        util::Result<drivers::SeekOutcome> r = drivers::SeekOutcome{};
        if (desc) {
            if (order_->scope().bottom.has_value()) {
                r = idx->seek_key(*order_->scope().bottom, true);
                if (r && !r.value().positioned) {
                    r = idx->seek_last();
                }
            } else {
                r = idx->seek_last();
            }
        } else {
            if (order_->scope().top.has_value()) {
                r = idx->seek_key(*order_->scope().top, true);
            } else {
                r = idx->seek_first();
            }
        }

        const std::uint32_t guard_max = driver_->record_count() + 1;
        std::uint32_t guard = 0;
        while (r && r.value().positioned && guard++ <= guard_max) {
            const std::string key = idx->current_key();
            if (!desc && !key_in_bottom_scope_(key)) {
                break; // Past the bottom scope in ascending order
            }
            if (desc && !key_in_top_scope_(key)) {
                break; // Past the top scope in descending order
            }
            if (key_in_top_scope_(key) && key_in_bottom_scope_(key)) {
                const std::uint32_t rc = r.value().recno;
                if (rc >= 1 && rc <= aof_bitmap_.size() && aof_bitmap_[rc - 1])
                    seq.push_back(rc);
            }
            r = desc ? idx->prev() : idx->next();
        }
        idx->invalidate_cursor();
    } else {
        seq.reserve(aof_bitmap_.size() / 4);
        for (std::size_t i = 0; i < aof_bitmap_.size(); ++i)
            if (aof_bitmap_[i]) seq.push_back(static_cast<std::uint32_t>(i + 1));
    }
    set_recno_sequence(std::move(seq));
}

[Admnwk]

Fixed in 90d1e96. rebuild_aof_sequence_ now filters rows by key_in_top_scope_/key_in_bottom_scope_ and breaks at the far boundary in the walk direction, guards driver_, and set_scope/clear_scope/clear_scopes rebuild the sequence. Added a regression test (AOF + index scope). I kept a full traversal with a per-row scope check + far-boundary break rather than a scope-boundary seek, to avoid soft-seek edge cases; happy to switch to the boundary seek if you prefer the O(scope) start.

[gemini-code-assist]

Prevent Potential Buffer Overflow in Test Code

In the test code, srv is allocated as a fixed-size buffer of 256 bytes, and std::memcpy is used to copy the directory path string into it. If the temporary directory path on the test machine or CI environment exceeds 255 characters, this will cause a buffer overflow.

Instead of copying to a fixed-size buffer, pass the path directly from std::string using a cast, which is both safer and matches the style used elsewhere in the codebase.

Suggested change

	UNSIGNED8 srv[256];
	std::memcpy(srv, dir.string().c_str(), dir.string().size() + 1);
	ADSHANDLE hConn = 0;
	REQUIRE(AdsConnect60(srv, ADS_LOCAL_SERVER, nullptr, nullptr, 0, &hConn) == 0);
	std::string srv_str = dir.string();
	ADSHANDLE hConn = 0;
	REQUIRE(AdsConnect60((UNSIGNED8*)srv_str.c_str(), ADS_LOCAL_SERVER, nullptr, nullptr, 0, &hConn) == 0);

[Admnwk]

Fixed in 90d1e96 — the connect path now passes the std::string directly via reinterpret_cast, no fixed-size buffer.

[Admnwk]

Withdrawing to re-verify against real-world data before resubmitting.