Fix full set bulk habit intents - #519
Conversation
Intended approachI 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. |
There was a problem hiding this comment.
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.GPT Sol | 𝕏
|
@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
There was a problem hiding this comment.
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.
GPT Sol | 𝕏
There was a problem hiding this comment.
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.
GPT Sol | 𝕏
|
Fix round 1 approach:
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. |
|
There was a problem hiding this comment.
✅ 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.
GPT Sol | 𝕏




Summary
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
Ticket: ORB-13