Skip to content

fix: tab session registry binding and fix-error prompt phrasing#1115

Merged
datlechin merged 2 commits intomainfrom
fix/test-infra
May 8, 2026
Merged

fix: tab session registry binding and fix-error prompt phrasing#1115
datlechin merged 2 commits intomainfrom
fix/test-infra

Conversation

@datlechin
Copy link
Copy Markdown
Member

Summary

Two fixes that landed during AI Wave 2 test cleanup. One is a real user-facing bug discovered while diagnosing test failures; the other tightens an internal contract that was masking test-only crashes.

What changed

fix(coordinator): bind tab session registry when coordinator owns the fallback (8f5853a9)

Production builds the TabSessionRegistry once in SessionStateFactory.create and hands it to both QueryTabManager AND MainContentCoordinator, so tabs.didSet keeps the registry in sync. When the coordinator falls back to creating its own registry (e.g. tests, or any composition that builds the coordinator without going through the factory), QueryTabManager had no way to know about it. tabs.didSet early-returned, no TabSession was registered, and restoreLastFilters tripped the assertionFailure at MainContentCoordinator+FilterState.swift:311.

Added QueryTabManager.bindTabSessionRegistry(_:). The coordinator calls it after resolving the fallback registry, which back-fills sessions for any pre-existing tabs. The assertion stays — it now only fires on real registry-sync regressions, which is what it was always supposed to do.

Also fixed the em-dash style violation in the assertion message.

fix(ai-chat): use display name and editor language for fix-error prompt phrasing (dec24036)

AIPromptTemplates.fixError always built its prompt as "This <queryLanguageName> query failed". For SQL that produces "This SQL query failed" (correct). For MongoDB it produced "This MQL query failed" (the AI doesn't always recognize MQL). For Redis it produced "This Redis CLI query failed" (Redis ops are commands, not queries).

queryInfo(for:) now switches on the plugin's editorLanguage and uses displayName for non-SQL languages, plus "command" instead of "query" for bash:

  • SQL: "SQL query" (unchanged)
  • JavaScript / others: "<DisplayName> query" ("MongoDB query")
  • Bash: "<DisplayName> command" ("Redis command")

This is a user-facing improvement — Fix Error prompts now read naturally for non-SQL backends, which improves the AI response quality.

Test plan

  • xcodebuild build green
  • swiftlint lint --strict clean on every touched file
  • Previously failing tests now pass: OpenTableTabTests/addsTabDirectlyWhenTabsEmptyNotSwitching, AIChatViewModelActionTests/fixErrorMongoDBConnection, AIChatViewModelActionTests/fixErrorRedisConnection
  • Manual smoke: open a MongoDB or Redis connection, run a query that errors, click "Fix Error", confirm the AI message reads naturally

Out of scope

Two pre-existing baseline failures unrelated to this PR:

  • MainContentCoordinatorLazyLoadTests/Returns early when tab is already executing
  • SessionStateFactoryTests/Connection-only payload without isNewTab creates empty tab manager

AIChatViewModelMentionsTests/currentQueryResolved also fails but is asserting on the wrong API (expects plainText to contain attachment text before resolveTurnForWire runs; sibling tests confirm raw-then-expanded is the intended design). Either the test or the assertion needs updating, separate from this PR.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin datlechin merged commit 1f6319f into main May 8, 2026
2 checks passed
@datlechin datlechin deleted the fix/test-infra branch May 8, 2026 06:44
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