Skip to content

Fix descendant search match ownership - #514

Merged
thomasluizon merged 3 commits into
mainfrom
fix/ticket-471-descendant
Sep 9, 2026
Merged

Fix descendant search match ownership#514
thomasluizon merged 3 commits into
mainfrom
fix/ticket-471-descendant

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Ancestors now report descendant title, description, and tag hits with the existing child reason and the matching descendant title.
  • A habit still reports description and tag only when its own fields matched.
  • Search result membership, completion gates, date gates, and overdue gates are unchanged.

Consumer conclusion

A consumer can now conclude that description and tag belong to the habit on the row, while child means the match belongs below that row. The child value names the matching descendant. No DTO field or contract shape changed.

Test evidence

  • With the defect present, dotnet test tests\Orbit.Application.Tests --filter "FullyQualifiedName~Handle_SearchMatchesDescendantField_ReturnsParentWithMatchMetadata" passed all 12 cases. The unchanged theory asserted the incorrect propagated field ownership.
  • With only the strengthened tests applied, dotnet test tests\Orbit.Application.Tests --no-restore --filter "FullyQualifiedName~Handle_SearchMatchesDescendantField_ReturnsParentWithMatchMetadata|FullyQualifiedName~Handle_SearchMatchesOnlyDescendantDescription_AttributesMetadataToOwningHabit|FullyQualifiedName~Handle_SearchMatchesOnlyDescendantTag_AttributesMetadataToOwningHabit" failed all 14 cases. Each failure showed an ancestor returning description or tag where child with value Exercise was expected.
  • After the resolver fix, the same 14 case command passed all 14 cases.
  • dotnet test tests\Orbit.Application.Tests --no-build --filter "FullyQualifiedName~SearchMatchesDescendant" passed all 24 descendant search cases, including completion, date, and overdue membership gates.
  • dotnet build Orbit.slnx completed with 0 errors. It emitted 16 existing dependency, analyzer compiler version, and Firebase deprecation warnings.
  • dotnet test passed 32 analyzer tests, 563 domain tests, 3,293 application tests, and 2,195 infrastructure tests.

Assumptions

  • The existing child reason with the descendant title is sufficient source identity. I rejected adding an optional DTO field because the visible mobile consumer already treats child as inherited and the ticket does not require rendering the descendant field type on the ancestor.
  • GitHub rejected creating a pull request while the branch matched main, so I used an empty planning commit to open the required preimplementation pull request. I rejected waiting until implementation because that would violate the repository sequence.

Related to https://github.com/thomasluizon/orbit-tickets/issues/471

@thomasluizon

Copy link
Copy Markdown
Owner Author

I will first inspect SearchMatchField and every match consumer visible in the API and mobile repository. The intended implementation is confined to src/Orbit.Application/Habits/Queries/HabitScheduleFilters.cs: each habit keeps description and tag reasons only for its own fields, while an ancestor receives the existing child reason when any descendant matches. Search membership remains unchanged. I will deliberately replace the old propagated-field expectation in tests/Orbit.Application.Tests/Queries/Habits/GetHabitScheduleQueryHandlerTests.cs and add focused descendant description and descendant tag cases. I expect to avoid a DTO change because the existing child reason expresses inherited ownership; I reject adding optional source identity unless consumer inspection shows that the descendant field reason must also be rendered on the ancestor, and I reject changing the returned habit set because that is outside ticket 471.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

This PR currently has no file changes, so the descendant search ownership defect remains unaddressed.

Reviewed changes Reviewed the sole commit and the existing descendant-search implementation and regression coverage.

  • Empty change set: 1f8a2bb changes zero files and leaves the base implementation and tests unchanged.

⚠️ The advertised fix is absent

The head is an empty commit directly on the base, so merging it cannot change match ownership. ComputeSearchMatches still recursively aggregates descendant matches into mapped ancestors, and the existing tests still assert that propagation rather than pinning the intended ownership.

Technical details
# Implement the descendant match ownership correction

## Affected sites
- `src/Orbit.Application/Habits/Queries/HabitScheduleFilters.cs:321``ComputeSearchMatches` still delegates to recursive descendant aggregation.
- `tests/Orbit.Application.Tests/Queries/Habits/GetHabitScheduleQueryHandlerTests.cs:655` — the current test expects descendant metadata on every node in the path.

## Required outcome
- Add the implementation that assigns match metadata to the intended owning habit while preserving the set of returned habits.
- Add regression coverage that fails under the current ownership behavior and verifies the returned habit set remains unchanged.

Pullfrog  | Fix it ➔View workflow run | Using GPT Sol𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the descendant match ownership implementation added since the prior Pullfrog review.

  • Corrected inherited ownership: Converted descendant title, description, and tag hits on ancestors to the existing child reason with the matching descendant title, while preserving each habit's own field metadata.
  • Strengthened recursive coverage: Updated depth and branch-gate assertions and added direct description-only and tag-only ownership cases that fail under the old propagation behavior.

Pullfrog  | View workflow run | Using GPT Sol𝕏

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the full descendant search ownership change, its recursive behavior, exact regression coverage, and the existing shared consumer semantics.

  • Corrected match ownership: Ancestors now receive the existing child reason with the matching descendant title for descendant title, description, and tag hits, while each matching habit retains its own field reason.
  • Preserved search gates: The metadata traversal continues to use the same completion, date, and overdue pruning as result membership.
  • Strengthened regression coverage: The updated depth theories and direct description and tag cases assert exact metadata at both ancestor and owning nodes.
  • Maintained consumer compatibility: The shipped shared schema already accepts child, and its badge helper renders that reason as an inherited sub-habit match without any DTO shape change.

Pullfrog  | View workflow run | Using GPT Sol𝕏

@thomasluizon
thomasluizon merged commit cb56382 into main Sep 9, 2026
23 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-471-descendant branch September 9, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant