Repository navigation
Conversation
Deploying quo with
|
| Latest commit: |
2afef49
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b2f8be23.quo-4k9.pages.dev |
| Branch Preview URL: | https://2-x-spike.quo-4k9.pages.dev |
Initial spike for Quo 2.x. Splits class-level and instance-level composition into physically different code paths: - Class composition (`Q1 + Q2` between classes) is unchanged. It still allocates a new anonymous Class via `Composing.composer`, suitable for type definition at code-load time. - Instance composition (`q1 + q2` between instances) now returns a value: an instance of one of two new concrete classes, `Quo::ComposedRelationBackedQuery` or `Quo::ComposedCollectionBackedQuery`, which hold the operands as Literal-typed instance state. Zero anonymous classes are allocated per call. `Composing.merge_instances` no longer dispatches into `Composing.composer`; it constructs the appropriate composed class directly. Pagination and transformer state are inherited from the operands (right wins for non-nil values), preserving the common 1.x runtime behaviour. The merge logic itself is unchanged — same Relation+Relation, Relation+Collection, Collection+Collection cases — factored into a shared `Quo::ComposedInstance` module that both new classes include. Benchmark on a representative composition (3-leaf chain, 5000 iterations of `(L.new + R.new + T.new).unwrap.to_sql`): | metric | main (1.0.0) | 2.x-spike | Δ | |---------------------|--------------|-----------|-------------| | wall | 3805 ms | 1740 ms | 2.2× faster | | GC time | 238 ms | 61 ms | 4× less | | total allocations | 8,000,091 | 4,755,036 | -41% | | T_CLASS allocations | 753 | 0 | -100% | Tests: 223/223 pass. The spike preserves the 1.x public API surface and behaviour, except for two intentional v2 semantic changes: 1. Instance composition no longer fans arbitrary user-defined props from a synthesised parent class. Each operand keeps its own constructor-time props. So `q2(spam_score: 0.5) + q3(spam_score: 0.9)` produces `(spam_score < 0.5) AND (spam_score < 0.9)` (the more restrictive filter), not "right wins". 2. `composed_instance.copy(some_user_prop: ...)` raises, because the composed instance's props are `(_left, _right, _joins, page, page_size, _specification)`, not the children's user-defined props. To "change" a child's prop, build a new operand with the new value and re-compose. Both changes were exercised by tests in 1.x; the affected tests are updated in this commit with comments describing the v2 semantics. The class-form `Quo::ComposedQuery` mixin and the `Composing.composer` machinery are untouched. Class composition keeps working unchanged. Design notes: design/V2.md. Status: spike. Not for merge into main yet. Open follow-ups in design/V2.md.
Composed instances now accept the same `copy(**overrides)` signature as leaf queries — including overrides for props that live on a child operand rather than on the composed instance itself. Algorithm: - Overrides matching the composed's own props (_left, _right, _joins, _specification, page, page_size) are passed straight to Literal's standard copy. - Overrides for any other key are walked right-first into the operand tree, recursing into composed operands, and applied to the first reachable Quo::Query operand that declares the prop. Composed operands recurse via the same fan_override path. - Unknown prop (declared by no operand) raises ArgumentError. Same surface as a normal copy(unknown_prop:) on a leaf. - Right wins on conflict — when both operands declare the same prop, only the right side is updated. Matches the rest of v2's "right wins" precedence (pagination, transformer). Subtle behavioural difference vs 1.x worth noting: 1.x's prop fan-out effectively reached every child that named the prop, because the merged class's prop hash was passed to each child at unwrap time. v2's set-once-on-right behaviour means only the right side gets the value; left keeps its original. Documented in design/V2.md as an open question — happy to flip to "fan to all matching operands" if that reads more naturally. Cost: O(tree size) per fan override. Not a hot path; documented as such. Tests: 230 runs / 692 assertions. Adds 7 cases covering own-prop copy, single-side fan-out, right-wins precedence, recursion into composed operands, AR::Relation operand skipped, unknown prop raises, and combined own+fan overrides.
Conceptually, a composed query exposes one logical `prop` — when you copy with a new value, it should land on every leaf that owns it. Switched from set-once-on-first-match to fan-to-all-matching-operands. Adds two recursion tests: - both operands are themselves composed queries, each with a leaf declaring the prop — the override walks each sub-tree and updates the leaf in each - 3-level-deep nesting on one side — the override still reaches the deepest leaf Right-precedence in 1.x was about *value selection* when both operands were constructed with the same prop name (`Q1(score: 0.5) + Q2(score: 0.7)` resolved to 0.7 across the merged class). In v2, each leaf keeps its own constructor-time value, so the merged relation is the AND of both. `copy` has no value-selection ambiguity to resolve: it sets the new value on every leaf that names the prop. 232 runs / 699 assertions.
…n tests
Closes three of the v2 open topics:
1. Specs on a composed instance.
`composed.order(...)`, `.joins(...)`, `.where(...)` already work — the
composed inherits `_specification` from RelationBackedQuery, and the
spec is applied to the merged relation at unwrap time. Operand-level
specs survive the merge as before. Both apply.
Added 4 regression tests covering: composed-only spec, composed + operand
specs combined, composed `.joins`, composed `.where`.
2. Transformer inheritance.
Confirmed and locked in 4 tests: right wins (the consistent v2
precedence), single-side inheritance from either operand, and outer
`.transform` on the composed wins over operand transformers.
3. `wrap` on hot paths — the bigger architectural one.
`Quo::RelationBackedQuery.wrap(rel).new` allocates a new anonymous
class per call, same shape of waste as 1.x instance composition.
Adds value-form constructors:
Quo::RelationBackedQuery.from(active_record_relation)
Quo::CollectionBackedQuery.from(enumerable)
Each returns an instance of a new concrete class
(Quo::WrappedRelationBackedQuery / Quo::WrappedCollectionBackedQuery)
with the wrapped relation/enumerable stored as a Literal prop. Zero
Class.new per call.
Tests assert 0 T_CLASS allocations for 100 successive `.from` calls,
and that `.from` interoperates with v2 instance composition (`+`
produces a Quo::ComposedRelationBackedQuery as expected).
`wrap` itself is unchanged. The class-returning form remains the right
shape for type-defining use (constant assignment, block form with
props). Existing call sites are unaffected. Migration is opt-in:
replace `wrap(rel).new` at call sites with `from(rel)`.
Performance contrast captured in a test: `wrap(rel).new` is shown
still allocating classes; `.from(rel)` doesn't.
252 tests / 753 assertions.
Closes topic 4 of the v2 open list. Pagination is a coupled (page, page_size) pair — the 1.x prop fan-out treated `page` and `page_size` as independent props, allowing a Frankenstein composed state where one operand contributed `page` and a different operand contributed `page_size`. v2 rule: a composed instance inherits its pagination as a unit from whichever operand is paginated, with right winning if both are. An operand is "paginated" iff its `page` is non-nil — `page_size` alone doesn't count because every Quo::Query has a default `page_size`. Behavioural change vs 1.x: the existing "composes_and_generates_valid_SQL_query" test asserted the Frankenstein behaviour (left's `page` paired with right's `page_size`); it's updated with a comment explaining what changed. Added 4 regression tests for the inheritance modes: - neither operand paginated → composed not paginated - only left paginated → composed inherits left's pair - only right paginated → composed inherits right's pair - both paginated → composed inherits right's pair (right wins) Also warmed up autoload in the existing `.from` zero-allocation tests so the first invocation's class-load doesn't inflate the T_CLASS count when these tests run before any other `.from` call. 256 runs / 764 assertions.
…y identity Closes topics 5 and 6 of the v2 open list. Topic 5 — remove unused instance strategies. In 2.x, instance composition (`q1 + q2`) constructs Quo::ComposedRelationBackedQuery or Quo::ComposedCollectionBackedQuery directly. The 1.x dispatch chain (InstanceStrategyRegistry + 4 InstanceStrategy subclasses) is unused on every code path. Deletes: - lib/quo/composing/instance_strategy.rb - lib/quo/composing/instance_strategy_registry.rb - lib/quo/composing/query_and_query_strategy.rb - lib/quo/composing/query_and_relation_strategy.rb - lib/quo/composing/relation_and_query_strategy.rb - lib/quo/composing/relation_and_relation_strategy.rb - test/quo/composing/instance_strategy_test.rb (test for deleted code) The remaining strategy infrastructure (BaseStrategy, ClassStrategy, ClassStrategyRegistry, QueryClassesStrategy) is still used by class composition (`Q1 + Q2` between classes) — untouched. Topic 6 — Quo::ComposedQuery mixin vs v2 concrete classes. No code change. Documented in design/V2.md that class-form and value-form composed queries are physically different types in 2.x. There is no unified marker module that's `kind_of?`-able across both, because they hold composition metadata at different layers (class ivars vs instance state). Users who need such a check across both forms can write a small helper — but it doesn't appear in any consumer code we can see. 251 runs / 755 assertions.
Closes topics 7 and 8 of the v2 open list. UPGRADING.md (top-level) Captures the two intentional behavioural changes for 1.x upgraders: - Instance composition no longer fans arbitrary user-defined props from a synthesised parent class. Each operand keeps its own constructor-time props. - Pagination inherits as a coupled (page, page_size) pair from whichever operand is paginated, with right winning if both are. Also documents what stays the same, what's new (`.from` value-form constructors), the perf numbers, and what to scan for after upgrading. Skill refresh Bumps the "Targets Quo `~> N`" banner from 1.0 to 2.0 across SKILL.md, README.md, and all five reference files. Adds: - Note in SKILL.md pointing 1.x upgraders at UPGRADING.md - SKILL.md "wrap vs from" section, including the `wrap(rel).new` anti-pattern. - COMPOSITION.md section on what instance composition returns (Quo::ComposedRelationBackedQuery / Quo::ComposedCollectionBackedQuery) and the `copy` fan-out behaviour. The install generator test is updated to expect the 2.0 banner. 251 runs / 755 assertions.
… queries
In v2, instance composition and the new `.from` constructors return
instances of concrete classes (Quo::ComposedRelationBackedQuery,
Quo::WrappedRelationBackedQuery, and the collection equivalents).
The first pass had these classes statically subclass Quo::RelationBackedQuery
and Quo::CollectionBackedQuery — bypassing the host application's
configured base classes (Quo.relation_backed_query_base_class /
Quo.collection_backed_query_base_class, typically ApplicationRelationQuery
/ ApplicationCollectionQuery). That dropped a 1.x-level feature: any
methods defined on the configured base class were available on
class-composed results but not on v2 value-form results.
Switches all four value-form classes to subclass the configured base
class instead:
class Quo::ComposedRelationBackedQuery < Quo.relation_backed_query_base_class
class Quo::ComposedCollectionBackedQuery < Quo.collection_backed_query_base_class
class Quo::WrappedRelationBackedQuery < Quo.relation_backed_query_base_class
class Quo::WrappedCollectionBackedQuery < Quo.collection_backed_query_base_class
The base class is resolved at autoload time. In Rails apps, initializers
(where `Quo.relation_backed_query_base_class_name = ...` lives) run
before eager load or first reference, so by the time these files load
the configured base is in place.
Adds four regression tests in test/quo/wrapped_query_test.rb covering:
- `.from` instance inherits from the configured relation base class
- `.from` collection instance inherits from the configured collection base class
- instance composition produces a value that inherits from the configured base
- collection instance composition inherits from the configured base
Each asserts `q.hello` resolves to the value defined on the dummy
app's ApplicationRelationQuery/ApplicationCollectionQuery.
255 runs / 763 assertions.
- Gemspec: `literal >= 1.9, < 2`. The 2.x line targets Literal 1.9+ (renamed `Literal::Null` → `Literal::Undefined`; updated internal type signatures). - Drop the defensive `LITERAL_UNSET` sentinel in `Quo::Query` — only `Literal::Undefined` is in play on >= 1.9. - Fix `Literal::Types::ConstraintType.new(range)` usage in relation_backed_query_test.rb (1.7+ signature change) — use the public `_Float(0...1.0)` constructor instead. - Strip narrative comment blocks from v2 files (composed_instance.rb, composed_relation_backed_query.rb, composed_collection_backed_query.rb, wrapped_relation_backed_query.rb, wrapped_collection_backed_query.rb, composing.rb). The user-facing facts live in UPGRADING.md and the bundled skill. - UPGRADING.md: note that composed/wrapped value-form instances subclass `Quo.relation_backed_query_base_class` and `Quo.collection_backed_query_base_class`, so behaviour added to those is available on `.from` and instance-composed values.
Bumps `activerecord` and `activesupport` deps to `>= 8.0, < 9` — Quo 2.x targets Rails 8 and above. For Rails 7.x users, stay on Quo 1.x. Issue #5 — joins / left_outer_joins splat Quo::RelationBackedQuerySpecification#joins and #left_outer_joins now accept multiple table arguments, mirroring ActiveRecord. apply_to handles single value or array. Issue #7 — Results delegation transforms returns too Block-style Enumerable methods on Quo::Results previously yielded transformed items into the block but returned the underlying (untransformed) items. Methods are now classified: - FILTER_METHODS (select, reject, find, sort_by, ...) -> return is transformed - PAIR_FILTER_METHODS (partition, minmax_by) -> each element of the pair is transformed - PASSTHROUGH_METHODS (any?, count, map, each_with_object, ...) -> return is verbatim (the block already received transformed items) New regression tests cover the post-fix shape across both RelationResults and CollectionResults. Issue #8 — strict wrap Quo::RelationBackedQuery.wrap raises ArgumentError at wrap time when given a positional argument that isn't an ActiveRecord::Relation or a Quo::RelationBackedQuery instance. Quo::CollectionBackedQuery.wrap similarly rejects non-Enumerable, non-Quo::CollectionBackedQuery positionals. Block forms still defer the type check until first call. Error messages point at the matching alternative constructor. Rails 8.1 SQL rendering Rails 8.1 renders boolean literals as FALSE / TRUE in sqlite SQL, where 8.0 renders 0 / 1. Tests now use a sql_false / sql_true helper that reads the value from the active connection's quote, so the suite passes on whichever Rails 8.x is bundled. 288 runs / 865 assertions.
The gemspec already requires `activerecord >= 8.0, < 9`, but Appraisals still included a `rails-7.2` entry which couldn't resolve against the new constraint. - Drop `rails-7.2` from Appraisals and the GitHub Actions matrix. - Add `rails-8.1` to both, so 8.0 and 8.1 are both tested. - Tighten `rails-8.0` to `~> 8.0.0`. - Regenerate gemfiles via `bundle exec appraisal install`. 288 / 865 on both 8.0 and 8.1.
Drops the leading-underscore prefix on the value-form props — they're
explicit constructor keyword args, not internal-only hacks:
- ComposedRelationBackedQuery: _left → left, _right → right, _joins → merge_joins
- ComposedCollectionBackedQuery: same
- WrappedRelationBackedQuery: _wrapped → wrapped
- WrappedCollectionBackedQuery: _wrapped → wrapped
`merge_joins` (rather than `joins`) for the composed-joins arg so it
doesn't shadow the fluent `.joins(...)` API from method_missing.
Tighten the prop types away from `Object`:
- Composed*.left/right: _Union(Quo::Query, Enumerable). Covers
Quo::Query, ::ActiveRecord::Relation (which includes Enumerable),
and bare Arrays / Enumerables for direct merge_instances callers.
- WrappedRelationBackedQuery.wrapped: ::ActiveRecord::Relation
- WrappedCollectionBackedQuery.wrapped: Enumerable
- merge_joins: _Nilable(_Union(Symbol, Hash, Array))
Bug fix:
- `_joins, default: -> {}` was being inspected as `lambda returning nil`
by standardrb (auto-fixed from `-> { nil }`). Functionally identical
(empty lambda body returns nil); previous audit incorrectly flagged
this as returning {}.
RBS / steep:
- Inline @RBS annotations updated for the renamed props and new types.
- Regenerated sig/generated/ via `bundle exec rbs-inline --output lib/`.
- Removed stale generated RBS for the 6 deleted instance-strategy files.
- `bundle exec steep check` passes.
Tests updated in test/quo/composed_query_test.rb and
test/quo/wrapped_query_test.rb for the new names (._right.foo →
.right.foo, etc.).
288 runs / 865 assertions, 0 failures. Lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- API_REFERENCE.md & UPGRADING.md: `Quo.relation_backed_query_base_class_name =` → `Quo.relation_backed_query_base_class =` (no `_name` suffix — the mattr_accessor is named without it; the value is the constant name as a String). - COMPOSITION.md & UPGRADING.md: `_left` / `_right` / `_joins` → `left` / `right` / `merge_joins` to match the actual props on `Quo::ComposedRelationBackedQuery` / `Quo::ComposedCollectionBackedQuery`. - COMPOSITION.md: transformer precedence on merge fixed from "left wins" to "right wins" (matches `Quo::Composing#inherited_transformer` and `UPGRADING.md`). - PAGINATION.md: pagination precedence between operands rewritten — was "leftmost wins"; correct rule is right-wins when both paginated, and pagination inherits as a coupled `(page, page_size)` pair (an operand with only `page_size` set is not paginated). - PAGINATION.md: `Defaults` preamble corrected — `page` has no default (stays nil), only `page_size` defaults. - PAGINATION.md & API_REFERENCE.md: `next_page_query` / `previous_page_query` warning — they require a non-nil `page` (compute `page + 1` / `page - 1` and raise on nil). - API_REFERENCE.md & SKILL.md: add fluent spec API section (`where`, `order`, `joins`, `distinct`, `includes`, `with`, `with_specification`, …) which routes through `method_missing` to `Quo::RelationBackedQuerySpecification`. - README.md: skill version banner `~> 1.0` → `~> 2.0`. - COMPOSITION.md: stale `1.0.0.beta3+` performance reference removed. - SKILL.md: clarified that `+` aliases differ by level — `compose` for classes, `merge` for instances. 288 runs / 865 assertions, lint clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.