[upadhyay-prashant]

Summary

CountStrategy rewrites count().is(P) to limit(n).count().is(P). When P accepts n, the capped value is emitted in place of the real count, so anything that observes it gets a wrong result:

• g.V().where(outE().count().is(gt(1)).is(lt(3))) keeps vertices with more than 2 edges
• g.V().outE().count().is(gt(2)) returns 3 instead of 6
• g.V().map(outE().count().is(gt(1))) returns 2 instead of 3

The limit is now only added when:

1. P rejects the cap (e.g. lt, lte, within), so any count that passes is exact; or
2. the count is used purely as a filter (where/filter, not, and/or, sideEffect, repeat().until()/emit()) and no step follows is(). A label at the end of a where() child adds a WhereEndStep that compares the count to the path, so that case is not limited.

The strategy also leaves count().is(P) alone when P contains a traversal (e.g. is(gt(1).and(lt(__.constant(3))))). That bound is only resolved at runtime, so no safe limit can be derived. This was already broken before this PR: filter(outE().count().is(gt(1).and(lt(__.constant(3))))) returned marko as well as josh, and outE().count().as("a").is(gt(1).and(lte(__.select("a")))) returned 2 instead of 6. A narrower rule (keep the limit when an and() has a literal part that rejects the cap) is possible as a follow-up.

Otherwise the traversal is left unchanged. The existing not() / filter-removal rewrites are untouched.

Testing

• CountStrategyTest: added cases covering shapes that must not be limited and shapes that must stay optimized; every case whose expectation changed has a still-limited counterpart (e.g. out().count().is(neq(4)) vs where(out().count().is(neq(4)))). Also covers traversal-valued predicates in root, filter-only, label-reading and not() forms.
• LazyBarrierStrategyTest / TraversalExplanationTest: updated two expectations that relied on the old root-level / by() rewrite; LazyBarrierStrategyTest also gained a where() counterpart.
• CountStrategy.feature: added 6 scenarios, all of which produced wrong results before the fix (except the lt(3).is(gt(1)) control) and pass after.
• Full gremlin-core suite and tinkergraph-gremlin (including its feature suites): 0 failures.
• Regenerated translations.json and the Go/Python/.NET/JS translation files (additions only). GLV feature tests were not run locally.

Notes

• Root-level and value-consuming usages such as count().is(gt(n)) no longer get the early limit(). That is required for correctness.
• This bug likely also exists on 3.7-dev/3.8-dev; this PR targets master.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.77%. Comparing base (a28cd1f) to head (2865a74).
⚠️ Report is 733 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3717      +/-   ##
============================================
+ Coverage     76.35%   76.77%   +0.41%     
- Complexity    13424    14365     +941     
============================================
  Files          1012     1037      +25     
  Lines         60341    64834    +4493     
  Branches       7075     7708     +633     
============================================
+ Hits          46076    49774    +3698     
- Misses        11548    11970     +422     
- Partials       2717     3090     +373     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Cole-Greer]

I think theoretically these cases could be optimized as

Suggested change

	{__.where(__.out().count().is(gt(1)).is(lt(3))), __.where(__.out().count().is(gt(1)).is(lt(3)))},
	{__.where(__.out().count().is(gte(2)).is(lte(2))), __.where(__.out().count().is(gte(2)).is(lte(2)))},
	{__.filter(__.out().count().is(neq(1)).is(lt(5))), __.filter(__.out().count().is(neq(1)).is(lt(5)))},
	{__.where(__.out().count().is(gt(1)).is(lt(3))), __.where(__.out().limit(3).count().is(gt(1)).is(lt(3)))},
	{__.where(__.out().count().is(gte(2)).is(lte(2))), __.where(__.out().limit(3).count().is(gte(2)).is(lte(2)))},
	{__.filter(__.out().count().is(neq(1)).is(lt(5))), __.filter(__.out().limit(5).count().is(neq(1)).is(lt(5)))},

I think the strategy is only looking at the count().is(gt/gte/neq).<some other step> pattern, which misses the context that the full chain of is() steps are all collectively constraining the count() in a way which could be optimized (the subsequent lt filter drops anything which would've exceeded the limit()). It's probably ok if we limit the scope of the strategy to the singular count().is() pattern, although I don't think these cases are necessarily warranted as they could be optimized further.

[Cole-Greer]

Thanks for all the updated @upadhyay-prashant. It looks great.

I've just got the one comment where I think we could optimize further by inspecting chains of is() steps, although I'm not sure that case really warrants the added complexity of the strategy. I'll defer to you whether you wish to implement that case.

VOTE +1

[Cole-Greer]

Nit: Shorter changelog entries are preferred, perhaps:

Suggested change

	* Fixed `CountStrategy` so that it no longer adds a `limit()` before `count().is(P)` when the capped count can pass the predicate and is then observed downstream, e.g. `where(out().count().is(gt(1)).is(lt(3)))` or `out().count().is(gt(2))`, or when `P` contains a traversal, e.g. `is(gt(1).and(lt(__.constant(3))))`.
	* Fixed `CountStrategy` so that it no longer adds a `limit()` before `count().is(P)` when the capped count can pass the predicate and is then observed downstream, e.g. `out().count().is(gt(2))`.