refactor: remove the InfiniteAIAssistant command type - #1624
Conversation
📝 WalkthroughWalkthroughThis PR retires the unreachable ChangesInfinite AI Assistant retirement
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
It shipped in 1.2.0 and was never creatable in any released version. Across all 202 tags the identifier only ever touched CommandType, the interface, the modal (+test), main.ts, packagePreview, MacroDisclosure, modelRefPinning, refreshStaleDefaultModelSeeds and one test - never a builder, an engine or an API file. The only add-AI path hardcodes CommandType.AIAssistant. Chunked prompting shipped instead as quickAddApi.ai.chunkedPrompt(), which is a strict superset: documented, tested, auto-splitting on provider rejection. Wiring it up was the alternative, and it is not a missing `if`: IInfiniteAIAssistantCommand has no promptTemplate and no text source, and ChunkedPrompt throws without a chunk reference. It would mean designing where the text comes from plus a template picker - a new feature with zero recorded demand, competing with a superset. If chunking ever returns to the builder, the right shape is a "chunk long input" toggle on the existing AI Assistant command reusing its promptTemplate, not a second command type. Removed: the enum member, IInfiniteAIAssistantCommand, the 268-line modal and its test, the systemPromptFields entry, the devMode testQuickAdd command (whose entire body constructed that modal) and its label, the packagePreview case, the MacroDisclosure label, the isAICommand clauses in modelRefPinning and refreshStaleDefaultModelSeeds, and the orphaned aiHelpers pair getMaxChunkTokensUpperBound / getLargestModelMaxTokens with their test, FALLBACK_MODEL_MAX_TOKENS and the estimateModelInputBudget import they were the only local users of. No data migration. Converting to a plain AIAssistant is not inert - with promptTemplate.enable false it opens a template picker and fires a billable request, or throws under disableOnlineFeatures (the default). Dropping the command deletes user data to fix a hazard nobody demonstrably has. And chunkSeparator is a string here but a RegExp in chunkedPrompt, so a stored "\n" is a literal backslash-n that no conversion could carry faithfully. The dispatch loop covers the hazard permanently and generically instead. The engine keeps telling the truth about the type after the enum member is gone: RETIRED_COMMAND_TYPES holds the message, so a data.json still carrying one gets "never runnable, now removed, use chunkedPrompt" rather than the generic "can come from a newer version of QuickAdd", which would be the exact opposite of what happened. Docs: the anchor-pinned "Max chunk tokens" section sat under AIAssistant's "Model settings and token budgets" next to real settings, but the only control that ever rendered it was the slider inside the unreachable modal. It moves to the chunkedPrompt API docs, reframed as the API option it actually is. The frozen anchor is preserved on both pages - the alias-anchor pattern this docs tree already uses - so /docs/AIAssistant#max-chunk-tokens still lands on a pointer to the new home. AGENTS.md pointed agents at the deleted quickadd:testQuickAdd command; it now points at quickadd:run and eval.
…r visible
Review follow-ups on the removal:
- The retired-type notice ran 438 characters, ~50% longer than any notice
QuickAdd has ever shipped, in a 15-second toast. The provenance clause ("no
macro-builder entry and no engine branch in any version") was commit-message
material with no user-side referent; the two actions now come first.
- The docs pointer was a bare `<a id>` under the "Max tokens is the context
window" section, so a reader arriving on the frozen anchor landed mid-way
through a section about a different field, with the phrase they clicked
nowhere on the page. It is a real heading again - same anchor, same ToC
entry, one line of body pointing at the API docs.
- AGENTS.md claimed `eval` takes "an expression, not statements". Statements
are fine and the completion value comes back; only top-level `await` fails.
Deploying quickadd with
|
| Latest commit: |
4a91cae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f66e443c.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-1571-remove-infini.quickadd.pages.dev |
ba78f0c to
4a91cae
Compare
|
Note on CI and review bots: CI ran by Once #1623 merges, retarget this PR to |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@AGENTS.md`:
- Around line 51-55: Update the `quickadd:run` and `eval` command examples in
AGENTS.md to include the required `vault=<vault>` selector as the command
prefix, while preserving their existing arguments and guidance about readiness,
completion status, and wrapping asynchronous eval work in an IIFE.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b3781d8e-484a-4862-aae9-fc6b2921e5b2
📒 Files selected for processing (22)
AGENTS.mddocs/src/content/docs/docs/AIAssistant.mddocs/src/content/docs/docs/QuickAddAPI.mdsrc/ai/AIAssistant.systemPromptLiteral.test.tssrc/ai/aiHelpers.maxChunkTokens.test.tssrc/ai/aiHelpers.tssrc/ai/modelRefPinning.tssrc/commandLabels.tssrc/engine/MacroChoiceEngine.tssrc/engine/MacroChoiceEngine.unknownCommand.test.tssrc/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.test.tssrc/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.tssrc/gui/PackageManager/MacroDisclosure.sveltesrc/gui/ai/systemPromptFields.test.tssrc/gui/ai/systemPromptLiteralNote.tssrc/main.commandLabels.test.tssrc/main.tssrc/migrations/refreshStaleDefaultModelSeeds.tssrc/services/packagePreview.test.tssrc/services/packagePreview.tssrc/types/macros/CommandType.tssrc/types/macros/QuickCommands/IAIAssistantCommand.ts
💤 Files with no reviewable changes (10)
- src/types/macros/CommandType.ts
- src/ai/aiHelpers.maxChunkTokens.test.ts
- src/main.commandLabels.test.ts
- src/types/macros/QuickCommands/IAIAssistantCommand.ts
- src/commandLabels.ts
- src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts
- src/gui/PackageManager/MacroDisclosure.svelte
- src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.test.ts
- src/ai/aiHelpers.ts
- src/main.ts
Stacked on #1623. Merge #1623 first - this branch contains its two commits, and the base flips to
masteronce it lands.What this removes, and why
CommandType.InfiniteAIAssistantshipped inae90edebon 2023-06-20, six minutes before the1.2.0tag. Every release since has carried it. It was never creatable in any released version: across all 202 tags,git log --all -S"InfiniteAIAssistant" --name-onlyshows the identifier only ever touchedCommandType.ts,IAIAssistantCommand.ts, the modal (+test),main.ts,packagePreview.ts,MacroDisclosure.svelte,modelRefPinning.ts,refreshStaleDefaultModelSeeds.tsand one test file. Never a builder, an engine, or an API file. NoInfiniteAIAssistantCommandclass ever existed; the only add-AI path isCommandSequenceEditor.ts -> new AIAssistantCommand(), which hardcodesCommandType.AIAssistant.It was abandoned deliberately. Chunked prompting shipped as a user script (discussion #535: "I'm kind of on the fence about whether it should be a feature") and later as
quickAddApi.ai.chunkedPrompt().Rejecting "wire it up instead." It is not a missing
if.IInfiniteAIAssistantCommandhas nopromptTemplateand no text source, andChunkedPromptthrows without a chunk reference. Wiring it up means designing where the text comes from plus a template picker: a new feature with zero recorded demand, competing againstquickAddApi.ai.chunkedPrompt, which is a strict superset (documented, 588-line test file, auto-split on provider rejection, 500-prompt cap). If chunking ever returns to the builder, the right shape is a "chunk long input" toggle on the existing AI Assistant command reusing itspromptTemplate, not a second command type. Recording that so it does not get re-proposed.Removed
Enum member ·
IInfiniteAIAssistantCommand· the 268-line modal + its 114-line test · thesystemPromptFieldsMODALSrow · the devModetestQuickAddcommand (its entire body constructed that modal - and passedtype: CommandType.AIAssistantfor anIInfiniteAIAssistantCommand, so even the dev harness built a mistyped object) and its label · thepackagePreviewcase · theMacroDisclosurelabel · theisAICommandclauses inmodelRefPinning.tsandrefreshStaleDefaultModelSeeds.ts· the orphanedgetMaxChunkTokensUpperBound/getLargestModelMaxTokenswith their test, plusFALLBACK_MODEL_MAX_TOKENSand theestimateModelInputBudgetimport they were the only local users of.chunkSeparator/resultJoiner/maxChunkTokens/mergeChunkson the script API path (AIAssistant.ts,quickAddApi.ts) are untouched. Same names, different feature - the biggest trap in this removal.No data migration
Deliberate, and the reasoning is worth keeping:
AIAssistantis not inert. WithpromptTemplate.enable: false,executeAIAssistant->runAIAssistantopens a prompt-template picker and fires a billable network request. It turns a no-op into an interactive, billable step the user never configured. UnderdisableOnlineFeatures(the default) it throws instead.chunkSeparatoris astringhere and aRegExpinchunkedPrompt, so a stored"\n"is a literal backslash-n.Verified in the live vault: a
data.jsonholding one survives load, all migrations, a macro-builder open/reorder/close, andsaveSettings()byte-identical (sha22cc0612…before and after). Nothing in this PR writes to it.Behaviour changes worth naming
1.
pinAiCommandModelRefsstops classifying it as an AI command at import time (packageImportService.ts->modelRefPinning.ts). Same forrefreshStaleDefaultModelSeeds. Both migrations now skip such a command instead of pinning amodelRef/ rewritingmodel/ stripping baked params on it. This removes writes rather than adding them, and the command was never runnable, so nothing observable changes - but it is a real behaviour change, not "2 LOC".2. Package import disclosure flips from a false promise to an honest warning.
KNOWN_COMMAND_TYPESisObject.values(CommandType), so removal moves a legacyInfiniteAIAssistantstep from the "AI assistant / Sends note content to your AI provider over the network" row to "Unknown capability. Review it manually." Pinned by a new test. It also meansMacroDisclosurerenders aCapabilityTagfor the row, never the now-label-less raw type string.3. The engine keeps telling the truth about the type after the enum member is gone. A naive removal would drop it into the generic branch and tell the user it "can come from a newer version of QuickAdd" - the exact opposite of what happened, and no help.
RETIRED_COMMAND_TYPEScarries the fact forward:The
outprompt behind it is the unchanged downstream symptom: the step still does not setout, so the capture still asks. What changed in #1623 is that the user is now told why. This PR adds what to do instead.4.
quickadd:runstill answersok: truefor a macro with a skipped step (the envelope already carriesverified: false). Enumerating skipped steps in the run envelope belongs with the #1606 result contract, not here.Docs
AIAssistant.mdhad an anchor-pinned### Max chunk tokens {#max-chunk-tokens}under Model settings and token budgets, sitting next to real settings - but the only "Max chunk tokens" control the plugin ever had was the slider inside the unreachable modal. A reader hunts for a setting that does not exist.The substance moves to the
chunkedPromptAPI docs, reframed as the API option it actually is, and the redundant duplicate bullet is folded into a pointer. Anchors are frozen, so{#max-chunk-tokens}is preserved as a real heading on both pages: the old page keeps its ToC entry and the term the reader clicked, with one line pointing at the new home.docs/scripts/check-links.py: 0 problems across 50 pages; bothid="max-chunk-tokens"anchors present in the built site.Also:
AGENTS.mdpointed agents at the deletedquickadd:testQuickAddcommand.Flagged, not folded in (sibling workers are active on #1607-#1615 in this area)
CommandList.svelte's{:else}fallback renders the retired command as an ordinary row with Delete + Reorder and no marker. Since there is deliberately no migration, the builder is where these live forever. Worth a follow-up - but the population is "users with a hand-editeddata.json", and the acquisition survey below found none.getModelMaxTokens(aiHelpers.ts) has no production caller either - dead onmaster, not introduced or adjacent here.Acquisition surface, for the record
Hand-edited
data.json, a script mutatingsettings.choices, or a hand-authored package. Zero hits in issues, zero in discussions, zero public JSON on GitHub, zero of the 6 public packages. That is "no evidence anyone has one", not "impossible" - which is exactly why the fix is #1623's generic guard rather than a migration.Verification
tsc -noEmitclean ·eslintclean ·svelte-check0 errors · 4644 tests pass · docs build + link check clean · live-verified in this worktree's isolated vault (legacydata.jsonround-trip, run-path notice,quickadd:testQuickAddgone from the command registry, remainingquickadd:*commands intact).Closes #1571
Summary by CodeRabbit
New Features
Bug Fixes
Refactor