fix: terminal node constraint projection in expansion step BED-7471

[urangel]

Adds terminal node constraint handling to buildExpansionPatternStep and refactors buildExpansionPatternRoot to use some of the same projection and constraint builder helper functions.

Using the ad example data zip with this query:
MATCH p = (:GPO)-[:GPLink]->(:Base)-[:Contains*1..]->(t:Base) WHERE COALESCE(t.system_tags, '') CONTAINS 'admin_tier_0' RETURN p LIMIT 1000

Before fix:

After fix:

Note: There are still some edges returned that do not have the terminal node constraint upheld in this after fix image. These edges seem to be partial matches of the entire result set and are to be handled as part of this task: https://specterops.atlassian.net/browse/BED-7468

Summary by CodeRabbit

• Refactor

  • Centralized projection and constraint handling in the query translation pipeline to improve maintainability and consistency.

• Tests

  • Updated translation and pattern-matching tests to reflect refined path seeding, recursion behavior, and projection/constraint outcomes.
  • Adjusted pattern expansion tests to use array-based kind filtering semantics.

[coderabbitai]

Walkthrough

Centralizes expansion projection and constraint logic in Go into new private builders and updates three SQL translation tests to adjust recursive aliasing, pattern-binding recursion/satisfaction, and node kind checks (switching to array containment).

Changes

Cohort / File(s)	Summary
SQL Translation Test Cases cypher/models/pgsql/test/translation_cases/multipart.sql, cypher/models/pgsql/test/translation_cases/pattern_binding.sql, cypher/models/pgsql/test/translation_cases/pattern_expansion.sql	Updated translation tests: multipart.sql adjusts recursive path aliasing and internal composite selection; pattern_binding.sql restructures base/recursive state, satisfaction propagation, and path assembly; pattern_expansion.sql replaces single-kind checks with array containment (kind_ids / pg_catalog.@>) across seed and recursive clauses.
Expansion Pattern Builder Refactor cypher/models/pgsql/translate/expansion.go	Refactors expansion projection/constraint assembly by adding private helpers buildExpansionPrimerProjection, buildExpansionRecursiveProjection, and buildExpansionProjectionConstraints; buildExpansionPatternRoot and step assembly now use these builders and centralize terminal/min-depth constraint handling. Method signatures updated to accept *ExpansionBuilder. Lines changed: +124/-170.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hop through queries, tidy every clause,
Primer and recursive projections in phrase,
Kinds now nest in arrays, neat and bright,
Constraints tucked in one cozy site,
I nibble bugs, and bound paths take flight.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: terminal node constraint projection in expansion step BED-7471' directly describes the main change: refactoring terminal node constraint handling in the expansion step with new helper functions for projection constraints.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch BED-7471

📝 Coding Plan for PR comments

• Generate coding plan

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cypher/models/pgsql/translate/expansion.go`:
- Around line 1055-1060: The error returns in
buildExpansionProjectionConstraints use the wrong type; change the error returns
in the branches where s.treeTranslator.ConsumeConstraintsFromVisibleSet and
ConjoinExpressions fail to return nil, err (not pgsql.Query{} , err) so the
function returns an pgsql.Expression-compatible nil on error; specifically
update the error returns inside the
expansionModel.TerminalNodeSatisfactionProjection block (the branches after
calling s.treeTranslator.ConsumeConstraintsFromVisibleSet and ConjoinExpressions
with pgsql.CompoundIdentifier{expansionModel.Frame.Binding.Identifier,
expansionSatisfied} and constraints.Expression) to return nil, err.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cf9f1d7d-06da-437b-8c4f-cc93ef3bf871

📥 Commits

Reviewing files that changed from the base of the PR and between 2838e86 and 3ba3cba.

📒 Files selected for processing (4)

• cypher/models/pgsql/test/translation_cases/multipart.sql
• cypher/models/pgsql/test/translation_cases/pattern_binding.sql
• cypher/models/pgsql/test/translation_cases/pattern_expansion.sql
• cypher/models/pgsql/translate/expansion.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cypher/models/pgsql/test/translation_cases/multipart.sql

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cypher/models/pgsql/translate/expansion.go`:
- Around line 1045-1075: In buildExpansionProjectionConstraints, always call
s.treeTranslator.ConsumeConstraintsFromVisibleSet(expansionModel.Frame.Visible)
and store its result in constraints (don't condition it on
expansionModel.TerminalNodeSatisfactionProjection); then set
projectionConstraints to constraints.Expression (or the consumed expression
wrapped appropriately) and only if
expansionModel.TerminalNodeSatisfactionProjection != nil conjoin
pgsql.CompoundIdentifier{..., expansionSatisfied} with that consumed constraints
via ConjoinExpressions (using the same ConjoinExpressions call currently
guarded), preserving the existing MinDepth optional conjoining logic that
follows.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 56eafea6-28e4-4319-a1e7-b3b2b8d6eb70

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba3cba and 2043e28.

📒 Files selected for processing (4)

• cypher/models/pgsql/test/translation_cases/multipart.sql
• cypher/models/pgsql/test/translation_cases/pattern_binding.sql
• cypher/models/pgsql/test/translation_cases/pattern_expansion.sql
• cypher/models/pgsql/translate/expansion.go

🚧 Files skipped from review as they are similar to previous changes (2)

• cypher/models/pgsql/test/translation_cases/pattern_binding.sql
• cypher/models/pgsql/test/translation_cases/multipart.sql

[seanjSO]

looks good to me!

[coderabbitai]

🧹 Nitpick comments (1)

cypher/models/pgsql/translate/expansion.go (1)

955-1040: Collapse the duplicated expansionSatisfied branches.

In both projection builders, the two branches only differ in the fourth select item. Pulling that slot into a small local/helper would reduce the chance of primer/recursive column order drifting the next time one branch changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cypher/models/pgsql/translate/expansion.go` around lines 955 - 1040, The two
projection builders (buildExpansionPrimerProjection and
buildExpansionRecursiveProjection) duplicate their branches only for the fourth
select item; introduce a local pgsql.SelectItem (e.g., expansionSatisfied :=
expansionModel.TerminalNodeSatisfactionProjection or pgsql.NewLiteral(false,
pgsql.Boolean)) at the top of each function (or a small helper that returns the
correct SelectItem) and use that variable in place of the differing fourth item
in both returned []pgsql.SelectItem slices so both branches share identical
ordering except for that single slot; ensure the chosen variable has type
pgsql.SelectItem and replace the literal/field in both the primer and recursive
return values.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cypher/models/pgsql/translate/expansion.go`:
- Around line 955-1040: The two projection builders
(buildExpansionPrimerProjection and buildExpansionRecursiveProjection) duplicate
their branches only for the fourth select item; introduce a local
pgsql.SelectItem (e.g., expansionSatisfied :=
expansionModel.TerminalNodeSatisfactionProjection or pgsql.NewLiteral(false,
pgsql.Boolean)) at the top of each function (or a small helper that returns the
correct SelectItem) and use that variable in place of the differing fourth item
in both returned []pgsql.SelectItem slices so both branches share identical
ordering except for that single slot; ensure the chosen variable has type
pgsql.SelectItem and replace the literal/field in both the primer and recursive
return values.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a50670c-c280-4cf3-9ea7-944263ae4dce

📥 Commits

Reviewing files that changed from the base of the PR and between 2043e28 and a19f528.

📒 Files selected for processing (1)

• cypher/models/pgsql/translate/expansion.go