Skip to content

Fix full set bulk habit intents - #519

Merged
thomasluizon merged 4 commits into
mainfrom
fix/ticket-19-bulk-intents
Sep 12, 2026
Merged

thomasluizon merged 4 commits into
mainfrom
fix/ticket-19-bulk-intents

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Resolves complete server-side habit sets for all, ID, tag, search, status, and frequency filters without model pagination.
  • Adds bounded generic bulk updates and rescheduling, and dispatches existing log, skip, and delete commands in bounded chunks so every match can be processed within command limits.
  • Replaces keyword emoji guesses and the placeholder fallback with bounded AI inference. Invalid or missing emoji mappings are skipped, successful batches are committed, and the legacy updated_count field remains an alias for committed applied_count.
  • Reports applied_count, total_matched, skipped_count, and partial from committed outcomes. Already logged habits do not inflate applied_count, and truncated AI or log results are surfaced explicitly as partial.
  • Prevents repeated single habit mutation loops and updates the prompt, MCP surface, registrations, capability catalog, and focused tests.

Scope decision

This pull request keeps both BulkRescheduleHabitsTool and BulkToolRepeatGuard. They are required for the ORB-13 behavior to be complete: an all-habits reschedule needs a dedicated bounded bulk path, and the dispatch guard prevents the same request from falling back to repeated single-habit calls. The sibling #20 is closed, and these pieces are tested and registered with the bulk execution path here as one independently mergeable change.

Red first evidence

SpecificEmoji_CommitsUpdatedHabitsBeforeReportingSuccess retains the assertion that SaveChangesAsync is received exactly once. Before the persistence fix, that assertion failed with zero calls, proving the missing-commit defect before the implementation was added.

Test evidence

  • dotnet build Orbit.slnx: succeeded with 0 errors and 8 pre-existing NU1608 warnings in the final incremental run.
  • dotnet test: 6,188 passed, 0 failed, 0 skipped.
    • Orbit.Analyzers.Tests: 32 passed.
    • Orbit.Domain.Tests: 582 passed.
    • Orbit.Application.Tests: 3,339 passed.
    • Orbit.Infrastructure.Tests: 2,235 passed.
  • Changed-file dash guard: passed.
  • Suppression allowlist guard: passed with all 101 declared sites accounted for.
  • Changed C# bare comment scan: no bare comments.

Ticket: ORB-13

@thomasluizon

Copy link
Copy Markdown
Owner Author

Intended approach

I will add filter keyed habit selection in Application so all and tag scoped operations resolve the complete owned set without routing identifiers through model pagination. Generic bulk updates and rescheduling will share that selector, execute through one bulk tool call, commit their writes, and return applied_count, total_matched, skipped_count where applicable, and partial.

Emoji inference will move behind a Domain interface implemented by the Infrastructure AI layer. It will send decrypted habit titles and descriptions in bounded batches using the subtask model tier, validate one emoji grapheme per returned habit identifier, skip malformed or missing mappings, stop honestly on failures or budget refusal, and commit each successful bounded batch. The current keyword table and sparkle fallback will be removed.

I will add the repeated identical single entity mutation gate in the chat tool dispatch path and strengthen the system prompt so multi entity requests select bulk tools and complete claims depend on tool coverage. I will also add explicit truncation metadata to the capped habit log read and surface length limited AI responses as partial.

Expected source locations are the chat bulk tool implementations and registrations under src/Orbit.Application and src/Orbit.Api, new Domain and Infrastructure AI service types, the existing HabitTools log response, the chat execution and prompt files, plus focused unit tests under tests. I am choosing shared server side filter resolution and bounded service batches over controller enumeration, raw SQL, or a larger keyword table because encrypted fields require the normal materialization path and complete counts must come from the server owned set.

@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 pull request contains no implementation, so it cannot fix the stated bulk habit intent behavior.

Reviewed changes I reviewed the complete pull request at cec844b; the head consists only of an empty starter commit.

  • Implementation: No files or production behavior are changed.
  • Tests: No regression coverage is added for the bulk habit intent bug.

⚠️ The stated bug remains unfixed

This non-draft pull request can currently merge without changing production behavior or adding regression coverage. Its Closes #19 line also points to unrelated, already-closed pull request #19 rather than identifying the intended ORB work.

Technical details
# Missing implementation

## Affected sites
- PR diff: the sole commit changes zero files.
- PR body: test evidence is still marked as pending implementation.
- PR body: `Closes #19` references an unrelated test-account pull request, and GitHub reports no closing issue for PR #519.

## Required outcome
- Add the intended bulk habit intent behavior and focused unit tests that fail with the diagnosed bug present.
- Link the actual ticket and replace the placeholder summary and test evidence before merge.

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

@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

The drift gate runs node tools/arch-map.mjs and fails on any diff. The new
BulkUpdateHabitsCommand, its validator and the emoji inference service changed
the handler and endpoint inventory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lvh6rf1XMFyVJ5hXZoMWhA

@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

The implementation resolves the previous empty-PR blocker, but four correctness gaps can over-select destructive targets, bypass habit consistency and entitlement checks, or report outcomes that do not match committed state.

Reviewed changes I reviewed the implementation added after the prior Pullfrog review at cec844b.

  • Added full-set selection and mutations: Introduced server-side habit filters plus generic bulk update and reschedule commands with chunked persistence and explicit result counts.

  • Routed complete action sets: Reworked bulk log, skip, and delete tools to select the complete matching set and dispatch existing bounded commands in chunks.

  • Replaced emoji inference: Added bounded AI inference, output validation, per-user usage attribution, and committed partial-result accounting.

  • Added orchestration safeguards: Blocked repeated single-habit writes, surfaced model and log truncation, registered chat and MCP operations, and expanded focused unit coverage.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using GPT Sol𝕏

Comment thread src/Orbit.Application/Habits/Commands/BulkHabitSelection.cs Outdated
Comment thread src/Orbit.Application/Habits/Commands/BulkUpdateHabitsCommand.cs Outdated
Comment thread src/Orbit.Application/Habits/Commands/BulkUpdateHabitsCommand.cs Outdated
Comment thread src/Orbit.Application/Chat/Tools/Implementations/BulkUpdateHabitsTool.cs Outdated

@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.

Caution

This PR can broaden an explicit bulk delete to the full filtered set and introduces update paths that can report or persist incorrect mutation outcomes.

Reviewed changes I reviewed the complete pull request at d91112e, including all implementation added after the starter commit.

  • Selection and dispatch: Adds server-side habit filters, chunked bulk operations, MCP wrappers, and repeated single-tool redirects.
  • Generic mutations: Adds shared field and schedule updates with applied, matched, skipped, and partial result counts.
  • Emoji inference: Replaces heuristic emoji selection with bounded AI inference and chunk commits.
  • AI response handling: Propagates truncation state, updates prompt rules, and registers the new tools and capability mappings.
  • Verification artifacts: Adds focused unit tests and regenerates the architecture map.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using GPT Sol𝕏

Comment thread src/Orbit.Application/Chat/Tools/Implementations/BulkHabitToolArguments.cs Outdated
Comment thread src/Orbit.Application/Chat/BulkToolRepeatGuard.cs Outdated
Comment thread src/Orbit.Application/Habits/Commands/BulkUpdateHabitsCommand.cs Outdated
Comment thread src/Orbit.Application/Habits/Commands/BulkUpdateHabitsCommand.cs Outdated
Comment thread src/Orbit.Application/Habits/Commands/BulkUpdateHabitsCommand.cs
Comment thread src/Orbit.Application/Habits/Commands/BulkUpdateHabitsCommand.cs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

Fix round 1 approach:

  1. Make bulk selection refuse simultaneous top level selectors and add an exact completion status predicate so destructive completed only requests cannot widen to active habits.
  2. Reload each update and emoji chunk inside every retry attempt, reset attempt counts there, and add committed counts only after the transaction succeeds.
  3. Run schedule updates under HabitCeilingLock, calculate the full chunk live root entry count under that lock, and ask IPayGateService for the whole allowance before mutation.
  4. Validate effective cadence before mutating a chunk, enforce the recurring quantity invariant in FluentValidation and the Habit entity, and route reminder transitions through ReminderStoreNormalizer.
  5. Preserve committed partial counts when a later chunk throws, and restrict repeat redirects to calls with equivalent mutation arguments.
  6. Add focused regressions for every behavior, including tests that fail first for selector ambiguity and retry reload semantics, then run build, the full suite, analyzer tests, and changed file comment checks.

The changes will stay in the existing bulk command, tool, selector, guard, validator, domain invariant, and focused test files. I will reuse HabitCeilingLock and ReminderStoreNormalizer instead of introducing parallel consistency or reminder logic. I will not broaden the bulk update contract to heterogeneous values because equivalent argument grouping fixes the reported guard defect without changing the API shape.

@sonarqubecloud

Copy link
Copy Markdown

@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 I reviewed the hardening changes added since the prior Pullfrog review at d91112e.

  • Tightened selection safety: Added explicit completed-state filtering and rejected conflicting non-null filter and legacy ID selectors before loading targets.
  • Preserved expressible updates: Limited repeated single-habit redirects to calls with equivalent mutation arguments.
  • Made committed outcomes reliable: Reloaded tracked entities inside retry attempts and retained exact partial counts when later chunks throw.
  • Restored consistency boundaries: Added the shared habit ceiling lock, live-root allowance checks, reminder-store normalization, and validator plus domain cadence guards.

Pullfrog  | View workflow run | Using GPT Sol𝕏

@thomasluizon
thomasluizon merged commit fb61d92 into main Sep 12, 2026
23 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-19-bulk-intents branch September 12, 2026 03:49
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