Harden Scout search, queue, and engine lifecycles - #464
Merged
Conversation
Reject zero and negative chunk sizes at the shared Database query owners before they can report false success or issue an unbounded query. Widen descending cursor pagination to accept string identifiers, matching forward traversal, and cover both query generation and real UUID-backed multi-page traversal.
Instantiate CollectedBy attributes instead of reading positional reflection arguments. This preserves constructor validation, supports named arguments and inherited overrides, and keeps the existing worker-lifetime resolution cache truthful.
Store Scout constraints as ordered field, operator, and value entries so repeated fields, comparisons, and soft-delete clauses survive every engine boundary. Resolve generic paginators through fresh parameterized container construction, honor database model-pagination capabilities in raw paths, and use the model default when per-page input is zero.
Consume structured comparisons consistently, preserve literal string-zero queries, and select soft-delete constraints by field instead of relying on the old associative shape. Instantiate search attributes so named and positional arguments behave identically, while retaining per-engine metadata caches and adding focused feature coverage.
Port configurable job attempts, backoff, exception limits, timeout behavior, and opt-in unique jobs while retaining Laravel-compatible subclass extension points. Give after-commit one top-level configuration owner and restore exact serialized Scout identities for queued removals without querying models that may already be deleted.
Make search suppression nesting-safe and coroutine-local, keep force deletion under one removal owner, add through-relation macros, and expose the observer methods every searchable implementation must provide. Resolve custom builders freshly, gate optional import events by listener presence, propagate the first concurrent child failure after draining active work, clear operation state in finally blocks, and use the unified after-commit setting.
Add validated ascending and descending queue imports for integer and string Scout keys while preserving configured queue and connection selection. Reject malformed or overflowing integer bounds, let the database own string collation, calculate platform-extreme ranges without numeric overflow, and return truthful command statuses.
Treat only missing or empty keys as absent so a literal zero reaches the engine, and confine unsupported-create handling to the create operation. Continue settings synchronization after unsupported creation, look up settings by logical key before physical index fallback, and allow operational failures to propagate unchanged.
Replace legacy numeric filter arrays with one escaped filters expression that preserves comparisons, booleans, grouped inclusion, exclusion, backed enums, and empty-set behavior. Preserve literal string-zero index overrides, retain request-scoped identify behavior, and delete queued models by their serialized Scout key.
Escape comparison and set filters consistently, preserve backed-enum values, and delete queued models by the exact serialized Scout key. Treat only HTTP 404 as an absent index, accept only the documented already-exists creation race, and propagate authentication, validation, transport, and other operational failures.
Resolve search and write targets explicitly, replace stale schema names with the selected operation target, and classify only documented missing and create-race errors as recoverable. Transform before I/O, import optimistically with one missing-collection recovery, avoid create-before-delete and create-before-flush calls, reject unsupported operators, and detach engine-owned collection wrappers to prevent worker-lifetime registry growth.
Add explicit void return types to the remaining Scout test methods whose results are not consumed. This completes the package typing pass without changing data providers, helper return contracts, or runtime behavior.
Declare the Guzzle and PSR HTTP message packages that Scout resolves directly instead of relying on root or optional engine dependencies to provide them transitively. Add a split-package metadata regression that verifies every direct runtime dependency and pins the intended constraints.
Record Scout provenance and the actionable Hypervel differences around queue configuration, coroutine-deferred indexing, bounded imports, search clients, and prefix-safe index deletion. Document comparisons, custom builders, queue job options, unique jobs, ordered imports, and Typesense collection naming in the existing Laravel-style user guide sections.
Capture the approved Scout design, upstream research, implementation boundaries, performance analysis, and validation plan in a package-specific record. Mark Scout complete in the framework routing and checklist records, record every durable finding, and close the Database cross-package revalidation entries.
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughScout adds structured search filters, directional queue imports, configurable and unique jobs, engine lifecycle corrections, model-aware builders, relation macros, and coroutine-safe cleanup. Database pagination and collection contracts also receive validation and type fixes. Unit and integration coverage validates the changes. ChangesScout parity and lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR updates Scout’s search constraints, queue behavior, model lifecycle handling, and external-engine resource management while preserving coroutine-aware execution.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/scout/src/Builder.php | Converts constraints to ordered structured clauses and correctly replaces the reserved soft-delete control entry through supported trash-selection APIs. |
| src/scout/src/Engines/DatabaseEngine.php | Applies structured comparison clauses and interprets the reserved soft-delete control predicate separately from ordinary model constraints. |
| src/scout/src/Engines/CollectionEngine.php | Mirrors DatabaseEngine’s structured filtering and soft-delete control behavior for in-memory searches. |
| src/scout/src/Console/ConcurrentImportRunner.php | Adds bounded concurrent import execution, first-failure propagation, draining, and coroutine-local cleanup. |
| src/scout/src/Console/QueueImportCommand.php | Extends queued range imports with validated ascending and descending traversal for integer and string identifiers. |
| src/scout/src/Engines/TypesenseEngine.php | Separates read and write targets, narrows missing-collection handling, and bounds SDK collection-wrapper retention. |
| src/scout/src/ModelObserver.php | Adjusts model lifecycle indexing behavior to avoid duplicate force-delete removal while retaining the intended delete path. |
| src/database/src/Concerns/BuildsQueries.php | Rejects invalid chunk sizes and supports descending cursor traversal for string identifiers. |
Reviews (3): Last reviewed commit: "fix(testing): await search service tasks" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/scout/src/Searchable.php (1)
104-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the shared listener-gated dispatch helper.
These two blocks repeat the logic of
SearchableScope::dispatchEvent()insrc/scout/src/SearchableScope.phpLines 75-82. Extract one helper and call it from both places. This keeps the listener gate and event construction in a single location.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/scout/src/Searchable.php` around lines 104 - 124, Extract a shared listener-gated event dispatch helper matching SearchableScope::dispatchEvent(), then replace the duplicated ModelsImported and ModelsFlushed listener checks in the HasManyThrough macros with calls to that helper. Keep event construction and dispatch inside the helper, and invoke it from both callback blocks.tests/Scout/Unit/Engines/TypesenseEngineTest.php (2)
208-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit assertion to the mock-only tests.
This test states its expectation only through
shouldNotReceive. PHPUnit marks a test risky when it records no assertion, unless the Mockery integration converts expectations into assertions. The neighbouring test at lines 196-206 adds$this->assertTrue(true)for this reason. Apply the same convention here and in the other new mock-only tests: lines 219-239, 241-271, 273-297, 343-368, 370-391, 467-483, 485-501, and 578-609.♻️ Proposed change
$this->createEngine($client)->update(new EloquentCollection([$model])); + + $this->assertTrue(true); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Scout/Unit/Engines/TypesenseEngineTest.php` around lines 208 - 217, Add an explicit $this->assertTrue(true) assertion to testUpdateTransformsBeforeTouchingTypesense and each listed mock-only test method, matching the neighboring test convention. Do not alter the existing Mockery expectations or test behavior.
853-907: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMove file-scope helper models and enums into test-specific namespaces. Six test files declare generic helper models and enums directly in the test file namespace. Names such as
TypesenseLifecycleModel,TypesenseStringCategory, andNamedSearchAttributesModelcan collide as more Scout tests are added. The coding guidelines require a test-specific namespace whose final segment is the test class name.
tests/Scout/Unit/Engines/TypesenseEngineTest.php#L853-L907: moveTypesenseLifecycleModel,TypesenseStringCategory, andTypesenseIntegerPriorityinto a namespace ending inTypesenseEngineTest.tests/Scout/Unit/Engines/MeilisearchEngineTest.php#L807-L836: moveMeilisearchTestChirpModel,MeilisearchStringStatus, andMeilisearchIntegerPriorityinto a namespace ending inMeilisearchEngineTest.tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php#L227-L254: moveTypesenseReadWriteSearchableModelandTypesenseCustomScoutKeyModelinto a namespace ending inTypesenseEngineIntegrationTest.tests/Integration/Scout/Typesense/TypesenseFilteringIntegrationTest.php#L160-L167: moveTypesenseFilterRankingandTypesenseFilterTitleinto a namespace ending inTypesenseFilteringIntegrationTest.tests/Integration/Scout/Meilisearch/MeilisearchFilteringIntegrationTest.php#L182-L190: moveMeilisearchFilterIdandMeilisearchFilterTitleinto a namespace ending inMeilisearchFilteringIntegrationTest.tests/Scout/Feature/DatabaseEngineTest.php#L389-L406: moveNamedSearchAttributesModelandMalformedSearchAttributeModelinto a namespace ending inDatabaseEngineTest.As per coding guidelines: "Use test-specific namespaces for collision-prone generic helper classes, with the test class name as the final namespace segment."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Scout/Unit/Engines/TypesenseEngineTest.php` around lines 853 - 907, Move the file-scope helpers into test-specific namespaces ending with their corresponding test class name, updating references and imports as needed: TypesenseEngineTest.php:853-907 for TypesenseLifecycleModel, TypesenseStringCategory, and TypesenseIntegerPriority; MeilisearchEngineTest.php:807-836 for MeilisearchTestChirpModel, MeilisearchStringStatus, and MeilisearchIntegerPriority; TypesenseEngineIntegrationTest.php:227-254 for TypesenseReadWriteSearchableModel and TypesenseCustomScoutKeyModel; TypesenseFilteringIntegrationTest.php:160-167 for TypesenseFilterRanking and TypesenseFilterTitle; MeilisearchFilteringIntegrationTest.php:182-190 for MeilisearchFilterId and MeilisearchFilterTitle; DatabaseEngineTest.php:389-406 for NamedSearchAttributesModel and MalformedSearchAttributeModel.Source: Coding guidelines
tests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.php (1)
66-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoth queued-removal tests can pass without indexing anything. Each test indexes a model, runs the serialized
RemoveFromSearchjob, and then asserts that the document is absent. If the indexing step silently stores nothing, the absence assertion still holds and the test reports success without exercising the removal path. Assert the indexed state before you run the job.
tests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.php#L66-L84: afterwaitForMeilisearchTasks(), assert that the index contains one hit, then run the job.tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php#L202-L224: afterupdate(), retrieve the document keyed by(string) $model->getScoutKey()and assert it exists, then run the job and expectObjectNotFound.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.php` around lines 66 - 84, Verify the document exists before exercising each queued-removal job. In tests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.php:66-84, after waitForMeilisearchTasks() and before RemoveFromSearch::handle(), assert the search results contain exactly one hit. In tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php:202-224, after update() and before running the job, retrieve the document using (string) $model->getScoutKey() and assert it exists; preserve the subsequent ObjectNotFound expectation.tests/Scout/Unit/Engines/MeilisearchEngineTest.php (1)
181-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the expected filter string so a mismatch reports a diff.
The assertion runs inside
m::on(). If the produced filter differs, Mockery reports "no matching handler" and does not show the actual string. The expected value contains nested quote and backslash escaping, so a mismatch is hard to diagnose. Capture the parameter and assert it after the call.♻️ Proposed refactor
+ $captured = null; + $index->shouldReceive('rawSearch') ->once() - ->with('query', m::on(function ($params) { - return $params['filter'] === 'status="active" AND score>=10 AND label!="draft\"review\\\copy" AND archived_at IS NOT NULL AND tags IN ["a\"b\\\c", true] AND hidden NOT IN ["draft", false]'; + ->with('query', m::on(function ($params) use (&$captured) { + $captured = $params['filter']; + + return true; })) ->andReturn(['hits' => [], 'totalHits' => 0]);Then assert after
$engine->search($builder);:$this->assertSame( 'status="active" AND score>=10 AND label!="draft\"review\\\copy" AND archived_at IS NOT NULL AND tags IN ["a\"b\\\c", true] AND hidden NOT IN ["draft", false]', $captured );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Scout/Unit/Engines/MeilisearchEngineTest.php` around lines 181 - 213, Update testSearchWithFilters to capture the rawSearch parameters in the m::on() callback instead of comparing the filter inline, then assert the captured filter with assertSame after $engine->search($builder). Keep the expected filter string unchanged so assertion failures report a direct diff.tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php (1)
186-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the schema name is not used as a collection target.
The test proves that indexing and searching work against
indexableAs(). It does not prove that the_stalename fromtypesenseCollectionSchema()is unused. The test passes even if the engine also creates the_stalecollection. Add an assertion that the_stalecollection does not exist.💡 Proposed additional assertion
$this->assertSame([$model->id], $results->pluck('id')->all()); + + $this->expectException(\Typesense\Exceptions\ObjectNotFound::class); + $this->typesense->collections[$model->searchableAs() . '_stale']->retrieve(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php` around lines 186 - 200, Extend testResolvedWriteTargetOverridesSchemaName to assert that the Typesense collection named from typesenseCollectionSchema() with the “_stale” suffix is absent after indexing and searching, while preserving the existing indexableAs() result assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scout/config/scout.php`:
- Around line 56-67: Cast the `after_commit` value in the Scout config using the
same boolean coercion as `soft_delete`, ensuring `SCOUT_AFTER_COMMIT` always
produces a consistent boolean for `Config::boolean(...)`. Also add an
upgrade/migration note documenting the replacement of `scout.queue.after_commit`
and `SCOUT_QUEUE_AFTER_COMMIT` with `scout.after_commit` and
`SCOUT_AFTER_COMMIT`.
In `@src/scout/src/Console/QueueImportCommand.php`:
- Around line 194-204: Update parseIntegerBound() to accept integral float
values and numeric strings returned by aggregate queries, converting valid
integer-valued inputs to int before returning. Preserve rejection of
non-integral, malformed, and out-of-range values, and keep
dispatchIntegerRange() receiving int|false bounds.
In `@src/scout/src/Contracts/SearchableInterface.php`:
- Around line 95-108: Add a Scout release note documenting the three new
SearchableInterface methods—searchIndexShouldBeUpdated(),
wasSearchableBeforeUpdate(), and wasSearchableBeforeDelete()—as a breaking
change for external implementations that do not use the Searchable trait, noting
that those implementations must add the methods.
In `@src/scout/src/Engines/AlgoliaEngine.php`:
- Around line 210-235: Update the where-clause mapping in AlgoliaEngine so
string values enter the formatting branch only for “=” and “!=” operators;
preserve other comparison operators such as “>”, “<”, “>=”, and “<=” for the
final filter expression or reject them explicitly instead of converting them to
“:”.
In `@src/scout/src/Engines/TypesenseEngine.php`:
- Around line 353-357: Update the TypesenseEngine where-filter mapping around
parseFilterValue() so null values are handled before being passed to its
non-nullable parameter. Reject null with a descriptive Scout-level exception
consistent with the MeilisearchEngine behavior, and apply the same handling to
the additional affected path.
In `@src/scout/src/Searchable.php`:
- Around line 236-247: Update Searchable::search to check whether the model
declares the static scoutBuilder property with property_exists() before reading
it; use the declared custom builder when present and Builder::class otherwise.
Preserve the existing makeWith arguments and search behavior.
In `@tests/Scout/Feature/SearchableModelTest.php`:
- Around line 33-51: Declare the static $scoutBuilder property on the Searchable
trait with the appropriate nullable builder-class type and default, so
Searchable::search() can safely evaluate static::$scoutBuilder ?? Builder::class
for models that do not override it.
In `@tests/Scout/Unit/BuilderTest.php`:
- Around line 587-627: Update
testGenericPaginationUsesFreshContainerSubstitutionsAndDefaultPerPageForZero to
restore or remove the Paginator and LengthAwarePaginator container bindings
after assertions, using a try/finally cleanup so it runs even when the test
fails and does not affect later tests.
In `@tests/Scout/Unit/Jobs/MakeSearchableTest.php`:
- Around line 70-84: Update UniqueByScoutKeys::uniqueId() to hash the existing
payload of model class and sorted Scout keys using seeded xxh128 instead of
sha256. Preserve the payload structure and seed, then update
testUniqueIdIsBasedOnTheClassAndScoutKeys() to compute the matching xxh128
expectation.
---
Nitpick comments:
In `@src/scout/src/Searchable.php`:
- Around line 104-124: Extract a shared listener-gated event dispatch helper
matching SearchableScope::dispatchEvent(), then replace the duplicated
ModelsImported and ModelsFlushed listener checks in the HasManyThrough macros
with calls to that helper. Keep event construction and dispatch inside the
helper, and invoke it from both callback blocks.
In `@tests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.php`:
- Around line 66-84: Verify the document exists before exercising each
queued-removal job. In
tests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.php:66-84,
after waitForMeilisearchTasks() and before RemoveFromSearch::handle(), assert
the search results contain exactly one hit. In
tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php:202-224,
after update() and before running the job, retrieve the document using (string)
$model->getScoutKey() and assert it exists; preserve the subsequent
ObjectNotFound expectation.
In `@tests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.php`:
- Around line 186-200: Extend testResolvedWriteTargetOverridesSchemaName to
assert that the Typesense collection named from typesenseCollectionSchema() with
the “_stale” suffix is absent after indexing and searching, while preserving the
existing indexableAs() result assertion.
In `@tests/Scout/Unit/Engines/MeilisearchEngineTest.php`:
- Around line 181-213: Update testSearchWithFilters to capture the rawSearch
parameters in the m::on() callback instead of comparing the filter inline, then
assert the captured filter with assertSame after $engine->search($builder). Keep
the expected filter string unchanged so assertion failures report a direct diff.
In `@tests/Scout/Unit/Engines/TypesenseEngineTest.php`:
- Around line 208-217: Add an explicit $this->assertTrue(true) assertion to
testUpdateTransformsBeforeTouchingTypesense and each listed mock-only test
method, matching the neighboring test convention. Do not alter the existing
Mockery expectations or test behavior.
- Around line 853-907: Move the file-scope helpers into test-specific namespaces
ending with their corresponding test class name, updating references and imports
as needed: TypesenseEngineTest.php:853-907 for TypesenseLifecycleModel,
TypesenseStringCategory, and TypesenseIntegerPriority;
MeilisearchEngineTest.php:807-836 for MeilisearchTestChirpModel,
MeilisearchStringStatus, and MeilisearchIntegerPriority;
TypesenseEngineIntegrationTest.php:227-254 for TypesenseReadWriteSearchableModel
and TypesenseCustomScoutKeyModel; TypesenseFilteringIntegrationTest.php:160-167
for TypesenseFilterRanking and TypesenseFilterTitle;
MeilisearchFilteringIntegrationTest.php:182-190 for MeilisearchFilterId and
MeilisearchFilterTitle; DatabaseEngineTest.php:389-406 for
NamedSearchAttributesModel and MalformedSearchAttributeModel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b89c5e4-5f19-4b81-bd6e-1f8ca20d42d8
📒 Files selected for processing (70)
docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-02-1006-scout-current-parity-queue-and-search-lifecycles.mdsrc/boost/docs/scout.mdsrc/database/src/Concerns/BuildsQueries.phpsrc/database/src/Eloquent/HasCollection.phpsrc/database/src/Query/Builder.phpsrc/scout/README.mdsrc/scout/composer.jsonsrc/scout/config/scout.phpsrc/scout/src/Builder.phpsrc/scout/src/Console/ConcurrentImportRunner.phpsrc/scout/src/Console/IndexCommand.phpsrc/scout/src/Console/QueueImportCommand.phpsrc/scout/src/Contracts/SearchableInterface.phpsrc/scout/src/Engines/AlgoliaEngine.phpsrc/scout/src/Engines/CollectionEngine.phpsrc/scout/src/Engines/DatabaseEngine.phpsrc/scout/src/Engines/MeilisearchEngine.phpsrc/scout/src/Engines/TypesenseEngine.phpsrc/scout/src/Jobs/MakeRangeSearchable.phpsrc/scout/src/Jobs/MakeSearchable.phpsrc/scout/src/Jobs/MakeSearchableUniquely.phpsrc/scout/src/Jobs/RemoveFromSearch.phpsrc/scout/src/Jobs/RemoveFromSearchUniquely.phpsrc/scout/src/ModelObserver.phpsrc/scout/src/Searchable.phpsrc/scout/src/SearchableScope.phpsrc/scout/src/Traits/ConfiguresJobOptions.phpsrc/scout/src/Traits/UniqueByScoutKeys.phptests/Database/DatabaseEloquentBuilderTest.phptests/Database/DatabaseEloquentModelTest.phptests/Database/DatabaseQueryBuilderTest.phptests/Database/Eloquent/Concerns/HasUuidsTest.phptests/Integration/Scout/Algolia/AlgoliaEngineIntegrationTest.phptests/Integration/Scout/Algolia/AlgoliaFilteringIntegrationTest.phptests/Integration/Scout/Meilisearch/MeilisearchEngineIntegrationTest.phptests/Integration/Scout/Meilisearch/MeilisearchFilteringIntegrationTest.phptests/Integration/Scout/Typesense/TypesenseEngineIntegrationTest.phptests/Integration/Scout/Typesense/TypesenseFilteringIntegrationTest.phptests/Scout/Feature/CollectionEngineTest.phptests/Scout/Feature/CoroutineSafetyTest.phptests/Scout/Feature/DatabaseEngineTest.phptests/Scout/Feature/SearchableModelTest.phptests/Scout/Feature/SearchableScopeTest.phptests/Scout/Models/TypesenseSearchableModel.phptests/Scout/PackageMetadataTest.phptests/Scout/ScoutTestCase.phptests/Scout/Unit/BuilderTest.phptests/Scout/Unit/ConfigFileTest.phptests/Scout/Unit/ConfigTest.phptests/Scout/Unit/Console/ConcurrentImportRunnerTest.phptests/Scout/Unit/Console/ImportCommandTest.phptests/Scout/Unit/Console/IndexCommandTest.phptests/Scout/Unit/Console/QueueImportCommandTest.phptests/Scout/Unit/EngineManagerTest.phptests/Scout/Unit/Engines/AlgoliaEngineTest.phptests/Scout/Unit/Engines/MeilisearchEngineTest.phptests/Scout/Unit/Engines/MeilisearchRetryMiddlewareTest.phptests/Scout/Unit/Engines/MeilisearchRetryPolicyTest.phptests/Scout/Unit/Engines/NullEngineTest.phptests/Scout/Unit/Engines/TypesenseEngineTest.phptests/Scout/Unit/Jobs/MakeRangeSearchableTest.phptests/Scout/Unit/Jobs/MakeSearchableTest.phptests/Scout/Unit/Jobs/RemoveFromSearchTest.phptests/Scout/Unit/MakeSearchableUsingTest.phptests/Scout/Unit/ModelObserverTest.phptests/Scout/Unit/QueueDispatchTest.phptests/Scout/Unit/ScoutServiceProviderTest.phptests/Scout/migrations/2025_01_01_000000_create_searchable_models_table.php
Normalize SCOUT_AFTER_COMMIT at the shipped config boundary so conventional 1 and 0 environment values satisfy the observer's strict boolean getter. Share the environment regression with the existing soft-delete case while preserving complete process-environment restoration. Validate Algolia's documented comparison operators before formatting and keep facet equality separate from numeric ordering. Reject null, non-finite, nonnumeric, and non-scalar ordering values locally; preserve native integer and finite-float rendering, boolean 1/0 semantics, and canonical numeric strings without precision loss. Strengthen queued-removal integration coverage by proving the exact custom Scout key exists in Meilisearch and Typesense before deletion. Update the Scout plan and audit ledger to record the final scalar, operator, config, and test boundaries.
Make Foundation's Meilisearch integration waiter inspect the terminal results it actually awaits and propagate failed tasks, timeouts, and transport errors instead of reporting false success. Consolidate Scout on that single waiter and use a Meilisearch-valid custom Scout key with a positive publication precondition. Wait for the exact Meilisearch and Algolia deletion tasks created during test cleanup so one test cannot race the next test's setup. Preserve loud setup failures, exception-safe teardown, and Algolia's global HTTP client restoration. Cover successful, failed, timed-out, and exact cleanup task ownership deterministically, document Meilisearch's identifier constraint, and record the Foundation-owned corrections in the audit ledger and dependency index.
Collaborator
Author
|
@greptileai @coderabbitai Follow-up is complete. The accepted review items are now addressed:
That positive precondition exposed two test-harness defects. The shared custom key used a character Meilisearch rejects, while the Meilisearch waiter discarded asynchronous task failures. The fixture now uses a valid identifier, Foundation owns the single truthful waiter, and both Meilisearch and Algolia cleanup wait for the exact deletion tasks they start. Setup fails when isolation cannot be established; teardown remains exception-safe. The rejected comments did not result in changes where the existing behavior was already correct or the proposed machinery would have weakened the design. The full repository gate and the configured Meilisearch integration group are green. |
|
Tip For best results, initiate chat on the files or code changes.
The config boundary now normalizes The cleanup and waiter changes also make the integration tests fail correctly when setup isolation fails. ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
Thanks for the thorough follow-up summary. The three accepted items are resolved in the right direction:
The two test-harness defects you found along the way (invalid Meilisearch key character, waiter discarding async task failures) are exactly the kind of defect that surfaces once a test actually exercises the real precondition rather than assuming it. Fixing them in Foundation so both engines share a single truthful waiter is the right ownership decision. The reasoning on the rejected comments is clear and the gate being green is the confirmation that matters. This is in good shape. |
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
1 participant
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.
Summary
This PR brings Scout up to the current Laravel-facing API while preserving Hypervel's coroutine-aware execution, shared search clients, bounded command concurrency, and existing engine extensions.
The main changes are:
For more details, see:
docs/plans/2026-08-02-1006-scout-current-parity-queue-and-search-lifecycles.md.Details
Query and model behavior
Scout constraints now retain the field, operator, and value as ordered entries. This preserves multiple comparisons against the same field and gives every engine the information it needs to render the query correctly.
Searchable models can select a custom Builder through the container. Generic pagination paths honor paginator substitutions without caching mutable instances. Raw Database pagination now uses the engine's declared model-pagination capabilities instead of treating a model paginator as a transport response.
The model lifecycle changes also prevent duplicate force-delete removal, restore nested search-suppression state correctly, and add the current HasManyThrough searchable and unsearchable macros.
Queue and command behavior
Search jobs read configured attempts, backoff, and exception limits. The opt-in unique jobs derive stable identities from the model class and sorted Scout keys. Queued removals reconstruct only the serialized search identity, so deletion remains correct after the database row is gone and does not perform a restoration query.
Queue imports support ascending and descending integer or string keys. Integer input is validated exactly, range arithmetic stays within platform integer bounds, and string ordering is left to the database collation. Concurrent imports retain and rethrow the first child failure after draining active work, then clear their coroutine-local runner state.
Engine behavior
Algolia now builds one escaped filters expression for comparisons, inclusion, exclusion, booleans, enums, and empty sets. Meilisearch applies the same value normalization and only treats a real not-found response as an absent index.
Typesense resolves read and write targets explicitly, ignores stale schema names in favor of the selected model index, transforms records before service access, and creates a collection only after a missing-collection response. Delete and flush no longer create first. Engine-owned collection wrappers are removed from the SDK registry after each operation, retaining the shared client and transport while preventing collection names from accumulating for the worker lifetime.
Database ownership
The shared chunk owners now reject zero and negative sizes instead of returning false success or issuing an unbounded query. Descending cursor pagination accepts string identifiers, and CollectedBy attributes are instantiated so named arguments and constructor validation work as intended.
Validation
composer fixgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation