[CI-5649] Add pre filter expression per section

[stanlp1]

Adds support for pre filter expression per section and corresponding tests.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for specifying a pre-filter expression per autocomplete section, enabling more granular scoping of results.

Changes:

• Extends autocomplete parameter typings with preFilterExpressionPerSection.
• Serializes per-section pre-filter expressions into autocomplete request query params.
• Adds a spec validating request serialization for preFilterExpressionPerSection.

Reviewed changes

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

File	Description
src/types/autocomplete.d.ts	Adds a new typed parameter for per-section pre-filter expressions.
src/modules/autocomplete.js	Appends per-section pre-filter expressions to the request query string.
spec/src/modules/autocomplete.js	Adds coverage for the new per-section parameter serialization.

---

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

[Copilot]

If both preFilterExpression and preFilterExpressionPerSection are provided, this code will emit two different shapes of the same pre_filter_expression query parameter (a scalar JSON string and bracketed per-section params). Depending on the query serializer and server-side parsing, this can lead to one value overriding the other or an invalid request. Consider enforcing mutual exclusivity (throwing/returning an error) or defining precedence (e.g., ignore preFilterExpression when preFilterExpressionPerSection is set) and serializing only one form.

[Copilot]

If both preFilterExpression and preFilterExpressionPerSection are provided, this code will emit two different shapes of the same pre_filter_expression query parameter (a scalar JSON string and bracketed per-section params). Depending on the query serializer and server-side parsing, this can lead to one value overriding the other or an invalid request. Consider enforcing mutual exclusivity (throwing/returning an error) or defining precedence (e.g., ignore preFilterExpression when preFilterExpressionPerSection is set) and serializing only one form.

[Copilot]

Manually constructing bracketed query keys is brittle and couples this logic to a particular querystring parsing/serialization convention. A more robust approach is to set queryParams.pre_filter_expression to an object keyed by section (with each value JSON-stringified if required by the API) and let the existing query serializer produce the correct wire format. This reduces the risk of mismatches if the querystring library or encoding rules change.

Suggested change

	if (preFilterExpressionPerSection) {
	Object.keys(preFilterExpressionPerSection).forEach((section) => {
	queryParams[`pre_filter_expression[${section}]`] = JSON.stringify(preFilterExpressionPerSection[section]);
	if (preFilterExpressionPerSection && !queryParams.pre_filter_expression) {
	queryParams.pre_filter_expression = {};
	Object.keys(preFilterExpressionPerSection).forEach((section) => {
	queryParams.pre_filter_expression[section] = JSON.stringify(preFilterExpressionPerSection[section]);

[constructor-claude-bedrock]

Code Review

This PR adds preFilterExpressionPerSection support to the autocomplete module, following the same pattern used by filtersPerSection. The implementation is clean and consistent with existing code. Two integration tests are added, but there are some gaps in test coverage and minor issues worth addressing.

Inline comments: 5 discussions added

Overall Assessment: ⚠️ Needs Work

[constructor-claude-bedrock]

Important Issue: Both new tests use the callback-style done pattern without a .catch() handler. If the promise rejects (e.g., a network or assertion error), done() is never called and the test times out with a confusing error rather than a meaningful failure message. Add a .catch(done) at the end of the chain, consistent with how the preFilterExpression test (just above) is written:

autocomplete.getAutocompleteResults(query, { preFilterExpressionPerSection })
  .then((res) => {
    // ...
    done();
  })
  .catch(done);

[constructor-claude-bedrock]

Suggestion: The assertion checks that the Products key exists on the parsed pre_filter_expression object, but the preFilterExpressionPerSection fixture only contains Products. An extra assertion verifying that no other section keys are present (i.e., expect(Object.keys(requestedUrlParams.pre_filter_expression)).to.have.lengthOf(1)) would guard against accidental leakage of additional parameters into the query. This is especially relevant as this is a new URL encoding scheme.

[constructor-claude-bedrock]

Suggestion: The existing preFilterExpression test (line ~394) also asserts on the response sections (e.g., res.sections.Products[0].data.facets), confirming the expression actually filters results. The new preFilterExpressionPerSection tests only check that the URL was formed correctly, but do not assert on the response contents. Consider adding assertions on the sections returned by the mock to verify end-to-end behavior, as done in the comparable test.

[constructor-claude-bedrock]

Suggestion: It would be valuable to add a test covering the interaction between preFilterExpression (the global one) and preFilterExpressionPerSection being passed simultaneously. Their query parameter keys are distinct (pre_filter_expression vs pre_filter_expression[Section]), so they could theoretically be combined in a single request. A test documenting that both parameters can coexist, or confirming they cannot, would make the contract explicit.