Repository navigation
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e6483b2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public class BigQueryParser extends AbstractSqlParser<MySqlParser, BigQueryDialect> { | ||
|
|
||
| public BigQueryParser() { | ||
| super(new MySqlParser(null), new BigQueryDialect()); |
There was a problem hiding this comment.
For valid BigQuery MERGE statements, switching to MySqlParser introduces a syntax regression because the MySQL grammar has no MERGE statement production, while the previous PostgreSQL grammar did. The new BigQueryCompletionSyntaxCoverageTest even codifies this regression with assertDoesNotParse, so editor parsing and syntax-dependent features will reject a standard BigQuery DML operation. Use a BigQuery-aware grammar or add the missing production and require MERGE to parse successfully in the focused dialect test.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
| static String resolve(boolean community, boolean localEdition) { | ||
| if (community) { | ||
| static String resolve(boolean windows, boolean community, boolean localEdition) { | ||
| if (windows && community) { |
There was a problem hiding this comment.
On the packaged Linux Community runtime, windows is false, community is true, and localEdition is false, so this condition falls through to Chat2DB Pro; MainJFrame.setupGenericPlatformSpecifics then applies that value as the window title. This misidentifies the Community package as the commercial product, and the added test currently codifies the incorrect title. Resolve Community identity independently of the OS and update the platform-focused test accordingly.
AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.
| if (workspaceStore.currentWorkspaceExtend === inspectorExtendCode) { | ||
| workspaceStore.setCurrentWorkspaceExtend(null); | ||
| } |
There was a problem hiding this comment.
After a user opens the row/value inspector and clicks its close button, this callback only clears currentWorkspaceExtend; layout.panelRight remains true, so WorkspaceRight continues allocating the saved panel width while WorkspaceExtendBody renders no content. The same blank sidebar can remain when selection or active-tab changes invoke this close path. Hide the right pane when no previous extension is being restored, or restore the prior right-pane content instead of leaving an open empty pane.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f65de8f09b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * is built on a grammar whose lexer recognizes backtick-quoted identifiers, combined with a | ||
| * BigQuery-specific {@link BigQueryDialect}, instead of reusing another engine's dialect rules. | ||
| */ | ||
| public class BigQueryParser extends AbstractSqlParser<MySqlParser, BigQueryDialect> { |
There was a problem hiding this comment.
When parserSqlScript(String) receives a common BigQuery query such as SELECT IF(flag, 1, 0); SELECT 2;, this class now inherits AbstractSqlParser's block handling, while BigQueryDialect inherits IF as a block-begin keyword but provides no rule manager to distinguish the scalar function. The resulting block never closes, so the semicolons are ignored and the splitter returns no statements; the previous PostgreSQL override did not apply these block rules. Supply BigQuery-specific block predicates or override the splitter, with focused coverage for IF(...) and END IF.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
| case SIMPLE -> new MysqlSimpleParserVisitor(statementContext); | ||
| case CREATE_TABLE -> new MysqlCreateTableVisitor(statementContext); | ||
| case VALID_TABLE -> new MysqlValidTableVisitor(statementContext); | ||
| default -> new MysqlParserVisitor(statementContext); |
There was a problem hiding this comment.
For SELECT * FROM project.dataset.table``, the lexer intentionally emits the quoted path as one token, but this default branch delegates semantic extraction to MysqlParserVisitor. Its `visitTableNameContext` treats a single-token `fullId` as a table whose name is the entire `project.dataset.table` string and leaves its database/schema unset; `DbSqlParserServiceImpl.contextParser` then defaults those scopes and tests `tableMap.containsKey(identifierName)`, whose keys are plain table names, so column mappings and hover/completion fail for the fully qualified identifiers this change is intended to support. Add a BigQuery visitor that maps the quoted project/dataset/table components into the identifier model and cover the resulting identifiers in a focused test.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
091d81d to
0c083d8
Compare
0c083d8 to
1fbd82f
Compare
…or-recovery fix(table): preserve filters after query errors
…ns-2601 feat(mysql): CSV encoding and format options (OtterMind#2601)
…fresh-errors fix(tree): preserve hidden settings and saved datasource state on refresh
…dact-2470 chore(ssh): redact credentials in SSHInfo.toString used by error logs
…-after-datasource-create fix(tree): expand namespace after datasource creation
…api-key-page chore(settings): remove unused API key page
…fo-copy fix(spi): preserve connection fields in copies
…s-null-guard-2505 fix(jdbc): guard getJarUrlsFromZip against null listFiles
…thread-2518 fix(cookie): make LocalCookie flush thread a named daemon
Keep completed task states and newly loaded history while polling. Invalidate stale responses on deletion or close and surface retryable load failures.
A prepared update that came from the beta channel failed verification in the next session: the remembered manifest was checked against the stable channel, so the manifest was declared untrusted and the downloaded beta update was discarded instead of being offered for installation. Verify the remembered manifest against the channel it names, exactly like the discovery verifies a manifest against the channel it queried. The manifest is covered by the signature, so its channel cannot be forged.
…lable" A check that could not reach the update source was reported exactly like a check that found no newer release, so a user with a broken network or a blocked download host was told "no new version available" and had no way to tell the difference. Report the two cases separately: - A release index or manifest that the source does not publish (HTTP 404) keeps meaning "no update available", because a channel that has not published its index yet is not an error. - Any other failure (connect timeout, server error, verification failure) is reported as a failed check. The check result carries a new CHECK_FAILED state, the JCEF handler maps it to the updateFailed status, and the client already renders that status as an error message. The new client message says the check failed and suggests checking the network instead of claiming that no new version exists.
…resume fix(updater): keep a downloaded update across checks and restarts
The community-release environment only carries the five macOS notarization and Feishu secrets, so a stable release runs every platform build with an empty COMMUNITY_UPDATE_KEY_ID and COMMUNITY_UPDATE_PUBLIC_KEY_B64 and fails at the first `test -n` in the packaging step. Stable and beta releases sign with the same update key, the same Windows signing connection and the same notarization credentials, so both channels use the community-beta-signing environment.
…ning-environment ci(release): use the shared signing environment for stable releases
A `-beta.<n>` tag used to create a GitHub prerelease, which only kept the build out of the release list: the Beta channel already separates clients through the update index. Publish it as an ordinary release whose notes say it is a Beta build instead. The resolve step reports `tag_push` (the stage step has to know whether the tag exists) and no longer reports `prerelease`; the publish step takes the Latest pointer from the channel, so Beta passes `--latest=false` and stable keeps `--latest`. Beta notes keep the generated change list and get the Beta statement prepended once.
ci(release): publish Beta builds as releases that keep Latest stable
The 5.4.0 line ships as an early preview before it reaches the regular Beta channel, so the README now says what that preview is, where to download it, and that Latest and the regular Beta line stay where they are. Every language version carries the same section with its own UI labels.
…ngs page The runtime selector in the AI input is where Pi Agent is enabled; there is no Settings section for it. Every language version now says so.
docs(readme): point at the 5.4.0 early preview
…iver KingBaseMetaData never overrode the list-level procedures(), so the call reached KbDatabaseMetaData#getProcedures, whose SPECIFIC_NAME expression "p.proname || '_' || p.oid" KingbaseES rejects with "operator does not exist: boolean || oid". The other PostgreSQL-family plugins answer this list themselves. Query sys_catalog.sys_proc the way the existing function list does, filtered to prokind = 'p', and map the rows to Procedure. KingBaseProcedureListTest drives the plugin through a proxy connection and fails if the driver path returns.
V8R3 tags every user routine 'u' and distinguishes a procedure only by its void return type, so the plain prokind = 'p' filter returned nothing there (procedures come back with prorettype 2278, a function returning int with 23). V8R6 and later keep using the 'p' marker, checked against KingbaseES V008R003 C002B0290 and V009R003C018.
…re-list fix(kingbase): list procedures from the plugin instead of the jdbc driver
The page installs, updates and locates the OtterMind SQLX command line, imports the datasources Chat2DB already holds, and shows the Skill commands an agent needs. SQLX is a separate product from the chat2db CLI, so it gets its own module and its own page section instead of extending the existing CLI entry. * chat2db-community-sqlx: platform and asset mapping for the official release, version parsing, a process runner, the release installer with SHA-256 verification and an atomic move, settings storage, and the status service. Credentials are read through queryDisplayDataSourceById(id, true) -- the decrypted view the connection path uses -- and travel to the SQLX child process on stdin, never through argv, files, the renderer or the logs. * chat2db-community-tools: the SqlxBridge contract, SqlxStatus and SqlxDatasourceState so the JCEF shell can reach the bridge without depending on a domain or storage module. * chat2db-community-jcef: eight handlers (status, install, update, check, cancel, choose binary, import, datasource states) plus a boundary test that fails if any handler imports an implementation package. * chat2db-community-start: registers the bridge before Spring starts and attaches the datasource reader after app.run, so the page never sees a missing bridge. * chat2db-community-client: the CLI 集成 settings section with the version line, one adaptive action button, a redetect that stays visible long enough to be seen, a datasource table with a status column (已导入 / 未导入 / 暂不支持 / 信息不完整) that only lets importable rows be selected, an import result reported as an alert, and the Skill commands with hover copy buttons. SqlxConnectionMapping now reaches the eight engines added for the next SQLX release (Presto, Hive, Kylin, XuguDB, Db2, Informix -- which Chat2DB spells INFOMIX -- SUNDB and GBase 8s), so they import once that release ships instead of being skipped as unsupported. Snowflake, Oscar, Elasticsearch, BigQuery and Redshift stay unsupported and keep reporting their reason. Verified: chat2db-community-sqlx tests (41, including the new engine mapping and the datasource reader that must use the decrypted view), test:sqlx-setting, test:settings-layout, test:i18n, eslint on the changed files, and build:web:community. The desktop app was exercised by hand against a real installation, an update and an import of a MySQL datasource.
A table data tab loads its first page from an effect that depended on the `viewTableParams` object. The workspace tab layer keeps every open tab mounted and rebuilds the tab bodies whenever unrelated workspace state changes (active tab, datasource list, tab list), so that object was new on most parent renders. React compares effect dependencies by reference, so every tab switch re-issued the browse request for every open table tab and pushed the visible page back to the first one. Load on the table identity (datasource, database, schema, table) and read the latest params from a ref instead. `workspaceTabItems` also no longer depends on the active tab id, which it never read, so switching tabs no longer rebuilds every tab body.
The Skill page section only showed `sqlx skill install --target <agent>`, which installs the Skill into an agent whose plugin the user still has to add. Each row now carries the command the SQLX README documents for that agent, so the plugin, the CLI and the Skill arrive together: * Codex and Claude Code: marketplace plus plugin install. * dsh: both documented profiles, browser UI and terminal UI. * pi: the npm package. The section description says the plugin brings the CLI and the Skill, and the settings search indexes plugin and marketplace. Verified with test:sqlx-setting, test:i18n, eslint and build:web:community.
The mapping filled `service` for Oracle only, so an imported Informix or GBase 8s connection stored an empty server instance. SQLX does not require it for those two engines, so the import reported success and the failure appeared later, when the driver refused the URL with "GBASEDBTSERVER has to be specified" or an INFORMIXSERVER error. Chat2DB keeps that instance in the form's service field (`InformixDBManager` appends `INFORMIXSERVER=`, `GBase8sDBManager` appends `GBASEDBTSERVER=`) and in the saved URL parameter block. The mapping now sends it as `--service`, and the database name it derives drops the `:KEY=value` block, which used to end up inside `database` (`app:INFORMIXSERVER=informix`). 42 module tests pass.
`useBinary` recorded the path, version and source of the adopted executable but left the note and digest of the download this page made. The page renders that note when the probe has no message of its own, so an adopted binary could still show "could not install into ..." from an earlier, unrelated install.
…egration Add the SQLX CLI integration to the settings page
Follow-up to the tab-switch load fix, from the review of that change: - Loading is now keyed on the table identity, so a cancelled or failed first page no longer recovers on its own. Surface that state inside the tab and give it a retry button instead of leaving the tab blank until it is reopened. - Add the missing rejection handler for the initial browse request, which was an unhandled rejection before. - `changeTabDetails` is captured by the memoized tab bodies, which are no longer rebuilt on a tab switch, so let `setWorkspaceTabsState` read the current split layout from the store instead of writing back an older one. - Document the effect-order invariant and the paging exception on the target key, and cover the database type and empty identity fields in its test.
The SQLX entry installs and drives a local command line through the desktop bridge, so a browser build cannot use it, and a commercial product layer must be able to hide it for a context it does not apply to. Two changes: * showSqlxSetting now follows the desktop bridge only. The community web build no longer offers an entry whose every action would reject. * The client extension gains `settings.useItems`, the settings counterpart of `mainPage.useNavigationItems`. The settings page always calls it through a default that keeps every entry, so Community behaviour is unchanged and a product layer can drop entries for the current context. Verified with test:settings-layout (assertions pin the contract), test:sqlx-setting, test:i18n and eslint.
…tab-refetch fix(workspace): stop table tabs from re-browsing on every switch
…isibility Hide the SQLX entry where its bridge is unavailable
- isolate the preset fetch failure in listAvailableModelOptions - keep the current list when a reload fails instead of clearing it - reload when the dropdown opens with an empty list (new onReloadModelOptions)
…t-resilience fix(ai): keep local model options when preset fetch fails
…nal callback An SSE stream could end after a business error payload without onSuccess, onError or onStop, so the hook stayed in LOADING and the assistant bubble kept spinning next to the error that had already arrived. - track whether a terminal callback fired and fall back to ERROR in the finally block, only while the request is still the current generation on a mounted component - stop the active request when the chunk handler surfaces an error
…-settle fix(ai): settle the stream status when a request ends without a terminal callback
Align existing OtterMind/Chat2DB PR OtterMind#2306 while preserving its focused fix.
3 participants
Fixes verified review finding: BigQueryParser was an empty subclass of PgsqlSqlParser (PostgreSQL grammar), so BigQuery-standard SQL (backtick-quoted identifiers like
my-project.dataset.table, QUALIFY/UNNEST) failed to parse in console/script splitting. Replaced with a self-contained BigQueryParser + BigQueryDialect (backtick identifiers tokenized, BigQuery start keywords incl. MERGE/DECLARE/SET/EXPORT). Regression tests added. Part of code-review findings tracked in OtterMind#2281.