Skip to content

Add SQL formatting and query plan analysis module via explain.tensor.ru API#9808

Open
MGorkov wants to merge 14 commits intopgadmin-org:masterfrom
MGorkov:master
Open

Add SQL formatting and query plan analysis module via explain.tensor.ru API#9808
MGorkov wants to merge 14 commits intopgadmin-org:masterfrom
MGorkov:master

Conversation

@MGorkov
Copy link
Copy Markdown

@MGorkov MGorkov commented Apr 1, 2026

This PR introduces a new module that integrates with the Explain PostgreSQL API (https://explain.tensor.ru/).
It provides users with enhanced SQL query formatting and in-depth query plan analysis directly within pgAdmin4.

Key Features:

External Plan Analysis: Send query plans to the external API for a more detailed analysis.
SQL Formatting: Utilize the API's capabilities to automatically format and beautify SQL queries.

Note:
This is a resubmission of the previously closed pull request [#9726] with all requested fixes applied.

Summary by CodeRabbit

  • New Features

    • Introduced Explain Tensor: interactive plan analysis panel, visualizations, smart recommendations, and SQL formatting via Explain Tensor API; Query Tool integrates Explain Tensor results.
  • Documentation

    • Added comprehensive Explain Tensor docs and updated developer tools/preferences pages with setup and usage guidance.
  • Chores

    • Added a master toggle to enable/disable Explain Tensor features.
  • Tests

    • Added unit tests covering Explain Tensor endpoints, URL validation, preferences, and formatting flows.
  • Translations

    • Added translation entries for Explain Tensor UI labels and messages.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

Walkthrough

Adds a new "Explain Tensor" feature: backend Flask blueprint with status, formatSQL, and explain endpoints; frontend React components and editor integration for formatting and plan analysis; configuration flag and preferences; documentation, translations, and unit tests.

Changes

Cohort / File(s) Summary
Documentation
docs/en_US/expl_tensor.rst, docs/en_US/preferences.rst, docs/en_US/developer_tools.rst
Added new Explain Tensor documentation page and preference section; registered page in developer tools toctree.
Configuration & Module Registration
web/config.py, web/pgadmin/tools/__init__.py
Added EXPLAIN_TENSOR_ENABLED config flag and registered the Explain Tensor blueprint during tools module registration.
Backend: Explain Tensor Module
web/pgadmin/tools/expl_tensor/__init__.py
New ExplTensorModule blueprint exposing /status, /formatSQL, /explain; includes URL validation, POST sender with 302 interception, preference retrieval helpers, and preference registration gated by config.
Frontend: Explain Tensor UI & Utilities
web/pgadmin/tools/expl_tensor/static/js/index.jsx, web/pgadmin/tools/expl_tensor/static/js/formatSQL.js
Added ExplainTensor React component (iframe-based result view) and async formatSQL utility that proxies through pgAdmin endpoints and handles 401 redirect flow.
Editor Integration & Events
web/pgadmin/static/js/components/ReactCodeMirror/index.jsx, web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js, web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx
Integrated async SQL formatting with loading state and Explain Tensor fallback; added QUERY_PLAN event and EXPLAIN_TENSOR panel constant; ResultSet now emits/handles plan events to open Explain Tensor panel.
Translations
web/pgadmin/messages.pot, web/pgadmin/translations/ru/LC_MESSAGES/messages.po
Updated POT header and source references; added translation entries and source refs for Explain Tensor UI strings and messages.
Tests
web/pgadmin/tools/expl_tensor/tests/... (__init__.py, test_status_endpoint.py, test_format_sql.py, test_explain_functionality.py, test_preferences.py, test_url_validation.py)
Added unit tests covering status, formatSQL, explain endpoint flows, URL validation, and preference retrieval with multiple scenarios and mocked dependencies.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant Editor as SQL Editor<br/>(ReactCodeMirror)
    participant API as pgAdmin API<br/>(expl_tensor)
    participant Service as Explain Tensor<br/>External Service

    User->>Editor: Request SQL format / Run EXPLAIN
    Editor->>API: POST /expl_tensor/formatSQL<br/>{query_src: sql}
    API->>Service: POST /beautifier-api
    Service-->>API: beautified_sql / error
    API-->>Editor: {success, data}
    Editor-->>User: Show formatted SQL

    Editor->>Editor: Execute query -> produce planJson
    Editor->>Editor: Emit QUERY_PLAN event (planJson, sql)
    Editor->>API: POST /expl_tensor/explain<br/>{plan, query}
    API->>Service: POST /explain (no-redirect mode)
    Service-->>API: 302 Location (relative_path) / error
    API-->>Editor: {success, data: {url}}
    Editor->>Editor: Open EXPLAIN_TENSOR panel, render ExplainTensor (iframe -> url)
    Editor-->>User: Display plan analysis
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.05% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main feature addition: a new SQL formatting and query plan analysis module that integrates with the explain.tensor.ru API.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (4)
web/pgadmin/tools/expl_pgsql/__init__.py (1)

239-254: Remove the unnecessary "Method" header.

The "Method": "POST" header on line 244 is non-standard and unnecessary. The HTTP method is determined by the request itself (via urllib.request.Request), not by a header. This header serves no purpose and could confuse the receiving server.

♻️ Proposed fix
     headers = {
         "Content-Type": "application/json; charset=utf-8",
         "User-Agent": "pgAdmin4/ExplainModule",
-        "Method": "POST"
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_pgsql/__init__.py` around lines 239 - 254, The headers
dict in send_post_request includes a non-standard "Method" header which should
be removed; update the headers used to build urllib.request.Request in
send_post_request to omit the "Method" key (keep "Content-Type" and
"User-Agent"), relying on urllib.request.Request/no302opener to set the POST
method via the presence of data, and verify behavior of no302opener.open(req,
timeout=10) remains unchanged.
web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx (1)

1042-1068: Missing dependencies in useEffect may cause stale closure issues.

The useEffect at lines 1042-1068 has an empty dependency array [] but references api, layoutDocker, and gettext in its callback. If any of these values change during the component's lifecycle, the event handler will use stale references.

However, based on the existing patterns in this file (e.g., line 1030-1040 for LLM status), this appears to be intentional since these values are expected to remain stable throughout the component's lifecycle.

💡 Consider adding dependencies for consistency
     eventBus.registerListener(QUERY_TOOL_EVENTS.QUERY_PLAN, showExplPgsql);
     return () => {
       eventBus.deregisterListener(QUERY_TOOL_EVENTS.QUERY_PLAN, showExplPgsql);
     };
-  }, []);
+  }, [api, layoutDocker]);

If api and layoutDocker are guaranteed stable (e.g., from context), the empty array is acceptable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx`
around lines 1042 - 1068, The useEffect registers showExplPgsql but uses
external symbols (api, layoutDocker, gettext) without listing them as
dependencies; fix by either adding api, layoutDocker, and gettext to the
dependency array of the useEffect that declares showExplPgsql (so the listener
always uses current references) or, if those values are truly stable (from
context/singleton), add a brief code comment and an eslint-disable-next-line
react-hooks/exhaustive-deps before the dependency array to silence the rule;
ensure you update the registration/deregistration using the same showExplPgsql
function name and keep eventBus and QUERY_TOOL_EVENTS references intact.
web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/formatSQL.js (1)

4-35: Avoid wrapping Promise constructor in an async function.

Since api.post already returns a Promise, you can simplify by using async/await directly rather than wrapping in new Promise(). The current pattern is an anti-pattern that can mask errors.

♻️ Proposed refactor using async/await
-export default async function formatSQL(sql) {
-  return new Promise((resolve, reject) => {
-    const api = getApiInstance();
-    api.post(
-      url_for('expl_pgsql.formatSQL'),
-      JSON.stringify({
-        query_src: sql,
-      }))
-      .then((res) => {
-        if (!res.data?.data) {
-          reject('No data returned from formatting API');
-          return;
-        }
-        try {
-          const {btf_query, btf_query_text} = JSON.parse(res.data.data);
-          if (btf_query !== btf_query_text) {
-            resolve(btf_query_text);
-          } else {
-            // Server returns identical text in both fields in case of an error
-            // In this scenario, we use the local formatter as a fallback
-            reject(btf_query_text);
-          }
-        } catch (err) {
-          console.error(err);
-          reject(err.message);
-        }
-      })
-      .catch((err) => {
-        console.error(err);
-        reject(err.message);
-      });
-  });
-};
+export default async function formatSQL(sql) {
+  const api = getApiInstance();
+  const res = await api.post(
+    url_for('expl_pgsql.formatSQL'),
+    JSON.stringify({
+      query_src: sql,
+    })
+  );
+
+  if (!res.data?.data) {
+    throw new Error('No data returned from formatting API');
+  }
+
+  const { btf_query, btf_query_text } = JSON.parse(res.data.data);
+  if (btf_query !== btf_query_text) {
+    return btf_query_text;
+  }
+  // Server returns identical text in both fields in case of an error
+  // In this scenario, we use the local formatter as a fallback
+  throw new Error(btf_query_text);
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/formatSQL.js` around
lines 4 - 35, The formatSQL function currently wraps api.post in a new Promise
(anti-pattern); refactor by using the existing async function to await the API
call: call getApiInstance().post(url_for('expl_pgsql.formatSQL'),
JSON.stringify({query_src: sql})) inside a try/catch, check res.data?.data,
JSON.parse it to extract btf_query and btf_query_text, return btf_query_text
when different and throw an Error with the fallback btf_query_text when
identical (server error case), and in catch log the error (console.error) and
rethrow (throw err or new Error(err.message)); reference functions/identifiers:
formatSQL, getApiInstance, url_for, api.post, and fields btf_query /
btf_query_text.
web/pgadmin/static/js/components/ReactCodeMirror/index.jsx (1)

150-150: formatSQL is now async but the keymap callback does not await it.

When formatSQL is invoked via the keyboard shortcut, it's called without await. While this may work due to the function's internal error handling, consider whether the UI should reflect the async nature or if errors should be surfaced differently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/static/js/components/ReactCodeMirror/index.jsx` at line 150, The
keymap is registering formatSQL via [parseShortcutValue(editorPrefs.format_sql,
true)] but the callback is not awaiting the now-async formatSQL; update the
keymap handler to be an async function that awaits formatSQL(...) and properly
handle/propagate errors (e.g., try/catch or rethrow) so the UI can reflect
completion or failures when invoking formatSQL from the keyboard shortcut.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/pgadmin/messages.pot`:
- Around line 16121-16124: Replace the typo "Postgresql" with "PostgreSQL" in
the user-facing message; find the exact string "Failed to post data to the
Explain Postgresql API" (used in the expl_pgsql module's __init__.py) and change
it to "Failed to post data to the Explain PostgreSQL API" in both places where
that literal is defined/returned so the .pot file will regenerate with
consistent capitalization.

In `@web/pgadmin/static/js/components/ReactCodeMirror/index.jsx`:
- Around line 103-119: The finally block incorrectly reads the stale state
variable loading; change to use a local flag/ref to detect whether the timeout
callback actually ran (e.g. let timeoutFired = false or use a React ref) and set
it to true inside the setTimeout callback where you call setLoading(true); then
in the finally block: if timeoutFired (meaning loading was set) call
setLoading(false) else clearTimeout(loadingTimeout). Update references around
explPgsqlFormatSQL, loadingTimeout, setLoading, loading, and formattedSql to use
this new flag so the timeout behavior and loading state are handled correctly.

In `@web/pgadmin/tools/expl_pgsql/__init__.py`:
- Around line 209-236: The docstring and comments claim SSRF prevention but
is_valid_url currently only checks scheme/hostname; update is_valid_url to
perform proper hostname/IP validation: parse with urlparse, extract
parsed.hostname, and for hostnames that are IP literals or resolve via
socket.getaddrinfo, check all resolved IPs with the ipaddress module and reject
any in RFC1918 (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), loopback
(127.0.0.0/8, ::1/128), link-local/metadata (169.254.169.254 and
169.254.0.0/16), and any other non-public ranges; only return True for
http/https when all resolved addresses are public/allowed. Update the docstring
to reflect the new SSRF checks and reference the is_valid_url function and
parsed.hostname behavior so reviewers can find the change.

In `@web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/index.jsx`:
- Around line 88-89: The propTypes declaration for ExplainPostgreSQL marks plans
as required (ExplainPostgreSQL.propTypes.plans = PropTypes.array.isRequired)
while the component also supplies a default value for plans (default props or a
parameter default like plans = []); remove the contradiction by either removing
the isRequired flag from ExplainPostgreSQL.propTypes.plans if you want the
default to cover missing props, or remove the default value (plans = [] or
defaultProps.plans) if plans must be provided by callers—update only one of
those locations to keep prop validation consistent.
- Around line 42-44: The request body is being JSON.stringified twice causing
the plan to be double-escaped; in ExplainPostgreSQL/index.jsx stop wrapping the
whole payload in JSON.stringify and send a plain object instead (e.g., { plan:
plans, query: sql }), and also stop stringifying plans — pass the plans variable
as an object so the backend's request.get_json() receives a proper dict; locate
the API call where JSON.stringify({ plan: JSON.stringify(plans), query: sql })
is used and replace it with a plain object payload.
- Around line 76-83: The iframe in ExplainPostgreSQL (index.jsx) currently uses
sandbox="allow-scripts", which can block interactive features of the external
explain.tensor.ru tool; test the embedded visualization with that domain and, if
interactions require it, update the iframe's sandbox attribute to include the
minimal additional permissions needed (e.g., allow-popups, allow-forms,
allow-modals) while keeping security in mind—locate the <iframe ...
sandbox="allow-scripts" ... /> in the ExplainPostgreSQL component and add only
the specific allow-* tokens required after testing.

In `@web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py`:
- Around line 35-90: The runTest method currently posts to the endpoint once
without mocks and then again inside the if self.method == 'POST' mocked block;
remove the first unmocked POST/assertion sequence (the initial self.tester.post
+ json.loads checks at the top of runTest) so the test only exercises the
endpoint within the with patch(...) nested mocks; keep the mocked block that
patches pgadmin.tools.expl_pgsql.get_preference_value, is_valid_url, and
send_post_request and preserve both expected_success branches there (using
json.dumps(self.data) for the POST when needed) so assertions still verify
response_data['success'] under the mocked conditions.

In `@web/pgadmin/translations/ru/LC_MESSAGES/messages.po`:
- Around line 16403-16406: The PO entry for the msgid "JSON payload must be an
object, not null, array, or scalar value" has an empty msgstr; open the
translations file and add the appropriate Russian translation into the msgstr
for that entry so the validation error is localized (target the PO entry
containing that exact msgid shown in the diff, which is referenced from
pgadmin/tools/expl_pgsql/__init__.py).
- Around line 16410-16418: Replace the misspelled Russian word "Некорретный"
with the correct spelling "Некорректный" in the translation entries for the
msgid "Invalid API endpoint URL. Only HTTP/HTTPS URLs are allowed." and the
multi-line msgid beginning "The provided API endpoint is not valid. Only
HTTP/HTTPS URLs are permitted." in messages.po so both msgstr occurrences are
corrected; search for those exact msgid strings (or inspect the translations
associated with pgadmin/tools/expl_pgsql/__init__.py references) and update the
two msgstr values accordingly.

In `@web/pgadmin/utils/__init__.py`:
- Line 386: Replace the string-concatenation logging call so it uses
parameterized logging: locate the current_app.logger.warning call that
concatenates 'Invalid binary path.' with binary_path and change it to pass the
format string and binary_path as parameters (use the current_app.logger.warning
method and the binary_path variable) so the message is logged as a parameterized
log entry rather than via string concatenation.

---

Nitpick comments:
In `@web/pgadmin/static/js/components/ReactCodeMirror/index.jsx`:
- Line 150: The keymap is registering formatSQL via
[parseShortcutValue(editorPrefs.format_sql, true)] but the callback is not
awaiting the now-async formatSQL; update the keymap handler to be an async
function that awaits formatSQL(...) and properly handle/propagate errors (e.g.,
try/catch or rethrow) so the UI can reflect completion or failures when invoking
formatSQL from the keyboard shortcut.

In `@web/pgadmin/tools/expl_pgsql/__init__.py`:
- Around line 239-254: The headers dict in send_post_request includes a
non-standard "Method" header which should be removed; update the headers used to
build urllib.request.Request in send_post_request to omit the "Method" key (keep
"Content-Type" and "User-Agent"), relying on urllib.request.Request/no302opener
to set the POST method via the presence of data, and verify behavior of
no302opener.open(req, timeout=10) remains unchanged.

In `@web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/formatSQL.js`:
- Around line 4-35: The formatSQL function currently wraps api.post in a new
Promise (anti-pattern); refactor by using the existing async function to await
the API call: call getApiInstance().post(url_for('expl_pgsql.formatSQL'),
JSON.stringify({query_src: sql})) inside a try/catch, check res.data?.data,
JSON.parse it to extract btf_query and btf_query_text, return btf_query_text
when different and throw an Error with the fallback btf_query_text when
identical (server error case), and in catch log the error (console.error) and
rethrow (throw err or new Error(err.message)); reference functions/identifiers:
formatSQL, getApiInstance, url_for, api.post, and fields btf_query /
btf_query_text.

In `@web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx`:
- Around line 1042-1068: The useEffect registers showExplPgsql but uses external
symbols (api, layoutDocker, gettext) without listing them as dependencies; fix
by either adding api, layoutDocker, and gettext to the dependency array of the
useEffect that declares showExplPgsql (so the listener always uses current
references) or, if those values are truly stable (from context/singleton), add a
brief code comment and an eslint-disable-next-line react-hooks/exhaustive-deps
before the dependency array to silence the rule; ensure you update the
registration/deregistration using the same showExplPgsql function name and keep
eventBus and QUERY_TOOL_EVENTS references intact.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e036953d-122a-4d50-8b4e-29c2a135a9bc

📥 Commits

Reviewing files that changed from the base of the PR and between d8a078a and 18a9e43.

⛔ Files ignored due to path filters (10)
  • docs/en_US/images/expl_pgsql_analyze.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_diagram.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_personal.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_piechart.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_plantree.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_recs.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_schema.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_stats.png is excluded by !**/*.png
  • docs/en_US/images/expl_pgsql_tilemap.png is excluded by !**/*.png
  • docs/en_US/images/preferences_expl_pgsql.png is excluded by !**/*.png
📒 Files selected for processing (19)
  • docs/en_US/developer_tools.rst
  • docs/en_US/expl_pgsql.rst
  • docs/en_US/preferences.rst
  • web/pgadmin/messages.pot
  • web/pgadmin/static/js/components/ReactCodeMirror/index.jsx
  • web/pgadmin/tools/__init__.py
  • web/pgadmin/tools/expl_pgsql/__init__.py
  • web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/formatSQL.js
  • web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/index.jsx
  • web/pgadmin/tools/expl_pgsql/tests/__init__.py
  • web/pgadmin/tools/expl_pgsql/tests/test_explain_functionality.py
  • web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py
  • web/pgadmin/tools/expl_pgsql/tests/test_preferences.py
  • web/pgadmin/tools/expl_pgsql/tests/test_status_endpoint.py
  • web/pgadmin/tools/expl_pgsql/tests/test_url_validation.py
  • web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js
  • web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.po
  • web/pgadmin/utils/__init__.py

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py (1)

37-65: ⚠️ Potential issue | 🔴 Critical

Critical bug: Test assertions execute outside the mock context.

The with patch(...) block exits at Line 46 (after setting up mock return values), but all test assertions (Lines 48-65) execute outside this context due to incorrect indentation. The mocks are not active when self.tester.post() is called, so tests hit the real endpoint or fail unpredictably.

🐛 Proposed fix: Move assertions inside the mock context
     def runTest(self):
         """Run test case."""
         with patch('pgadmin.tools.expl_pgsql.get_preference_value') as mock_pref, \
               patch('pgadmin.tools.expl_pgsql.is_valid_url') as mock_valid, \
               patch('pgadmin.tools.expl_pgsql.send_post_request') as mock_send:
 
             mock_pref.return_value = 'https://explain.tensor.ru'
             mock_valid.return_value = True
             mock_send.return_value = (False, json.dumps({
                 'btf_query': '<span class=\'sql_keyword\'>SELECT</span>',
                 'btf_query_text': 'SELECT\\n\\t*\\nFROM\\n\\ttest_table\\nWHERE\\n\\tid = 1;'
             }))
 
-        if self.expected_success:
-            response = self.tester.post(
-                self.url,
-                data=json.dumps(self.data),
-                content_type='application/json'
-            )
-            self.assertEqual(response.status_code, 200)
-            response_data = json.loads(response.data.decode('utf-8'))
-            self.assertTrue(response_data['success'])
-        else:
-            response = self.tester.post(
-                self.url,
-                data=self.data,
-                content_type='application/json'
-            )
-            self.assertEqual(response.status_code, 200)
-            response_data = json.loads(response.data.decode('utf-8'))
-            self.assertFalse(response_data['success'])
+            if self.expected_success:
+                response = self.tester.post(
+                    self.url,
+                    data=json.dumps(self.data),
+                    content_type='application/json'
+                )
+                self.assertEqual(response.status_code, 200)
+                response_data = json.loads(response.data.decode('utf-8'))
+                self.assertTrue(response_data['success'])
+            else:
+                response = self.tester.post(
+                    self.url,
+                    data=self.data,
+                    content_type='application/json'
+                )
+                self.assertEqual(response.status_code, 200)
+                response_data = json.loads(response.data.decode('utf-8'))
+                self.assertFalse(response_data['success'])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py` around lines 37 - 65,
The test assertions are running outside the patch context so mocks
(patch('pgadmin.tools.expl_pgsql.get_preference_value'),
patch('pgadmin.tools.expl_pgsql.is_valid_url'),
patch('pgadmin.tools.expl_pgsql.send_post_request')) are not active when
self.tester.post(...) is invoked; fix this by moving the branches that call
self.tester.post(...) and the subsequent assertions (the blocks that reference
self.url, self.data, self.expected_success and inspect
response.status_code/response_data['success']) so they execute inside the with
patch(...) as mock_pref, ...: block, ensuring mock_pref.return_value,
mock_valid.return_value and mock_send.return_value are in effect during the POST
and assertions.
🧹 Nitpick comments (2)
web/pgadmin/tools/expl_pgsql/__init__.py (1)

138-147: Variable data is reused, reducing readability.

The data variable is first the request payload (Line 112), then overwritten with the API response (Line 139). While functionally correct, this shadowing reduces code clarity.

♻️ Proposed fix for clarity
     api_url = explain_postgresql_api + '/beautifier-api'
-    is_error, data = send_post_request(api_url, data)
+    is_error, response_data = send_post_request(api_url, data)
     if is_error:
         return make_json_response(
             success=0,
-            errormsg=data,
+            errormsg=response_data,
             info=gettext('Failed to post data to the Explain PostgreSQL API'),
         )
 
-    return make_json_response(success=1, data=data)
+    return make_json_response(success=1, data=response_data)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_pgsql/__init__.py` around lines 138 - 147, The code
reuses the variable name data for both the outgoing payload and the API response
which reduces readability; change the original request payload variable
(currently named data at its definition and passed into send_post_request) to a
clearer name like payload or request_payload, and assign the result of
send_post_request to a different variable (e.g., response_data or api_response),
then update the make_json_response calls and the send_post_request invocation to
use these new names so that send_post_request(api_url, request_payload) returns
(is_error, response_data) and the final return uses response_data.
web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py (1)

12-12: Unused import: unittest.

The unittest module is imported but not used since the test class extends BaseTestGenerator.

♻️ Proposed fix
-import unittest
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py` at line 12, Remove the
unused import of the unittest module from
web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py; the test class inherits
from BaseTestGenerator (not unittest.TestCase) so delete the line "import
unittest" and ensure there are no remaining references to unittest in the file
(update imports if needed to keep only used test helpers).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/pgadmin/tools/expl_pgsql/__init__.py`:
- Around line 199-206: The code currently rejects Location values that are
absolute URLs; update the handling around response_data and res_data so absolute
URLs from Location are accepted: if response_data is an absolute URL (e.g.,
startswith('http://') or 'https://') set res_data = response_data, otherwise
keep the existing behavior of concatenating explain_postgresql_api +
response_data; keep using make_json_response(success=1, data=res_data) and
retain the error branch only for completely unexpected formats (neither absolute
URL nor a relative path starting with '/'); reference response_data and
explain_postgresql_api when making this change.

In `@web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/index.jsx`:
- Around line 37-59: The effect in useEffect uses the variables plans and sql
(and calls getApiInstance/post) but only lists plans in its dependency array,
causing stale sql to be sent when sql changes; update the dependency array for
the useEffect that contains the API call (the block that posts to
url_for('expl_pgsql.explain') and calls setData/setError/setIsLoading) to
include sql (and any other external stable references if needed) so the effect
reruns when sql changes.

---

Duplicate comments:
In `@web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py`:
- Around line 37-65: The test assertions are running outside the patch context
so mocks (patch('pgadmin.tools.expl_pgsql.get_preference_value'),
patch('pgadmin.tools.expl_pgsql.is_valid_url'),
patch('pgadmin.tools.expl_pgsql.send_post_request')) are not active when
self.tester.post(...) is invoked; fix this by moving the branches that call
self.tester.post(...) and the subsequent assertions (the blocks that reference
self.url, self.data, self.expected_success and inspect
response.status_code/response_data['success']) so they execute inside the with
patch(...) as mock_pref, ...: block, ensuring mock_pref.return_value,
mock_valid.return_value and mock_send.return_value are in effect during the POST
and assertions.

---

Nitpick comments:
In `@web/pgadmin/tools/expl_pgsql/__init__.py`:
- Around line 138-147: The code reuses the variable name data for both the
outgoing payload and the API response which reduces readability; change the
original request payload variable (currently named data at its definition and
passed into send_post_request) to a clearer name like payload or
request_payload, and assign the result of send_post_request to a different
variable (e.g., response_data or api_response), then update the
make_json_response calls and the send_post_request invocation to use these new
names so that send_post_request(api_url, request_payload) returns (is_error,
response_data) and the final return uses response_data.

In `@web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py`:
- Line 12: Remove the unused import of the unittest module from
web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py; the test class inherits
from BaseTestGenerator (not unittest.TestCase) so delete the line "import
unittest" and ensure there are no remaining references to unittest in the file
(update imports if needed to keep only used test helpers).
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f70db9a2-d6b6-44af-b7d4-a3a8669c3649

📥 Commits

Reviewing files that changed from the base of the PR and between 18a9e43 and e4f886a.

📒 Files selected for processing (7)
  • web/pgadmin/messages.pot
  • web/pgadmin/static/js/components/ReactCodeMirror/index.jsx
  • web/pgadmin/tools/expl_pgsql/__init__.py
  • web/pgadmin/tools/expl_pgsql/static/js/ExplainPostgreSQL/index.jsx
  • web/pgadmin/tools/expl_pgsql/tests/test_format_sql.py
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.mo
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.po
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.po
  • web/pgadmin/messages.pot

@asheshv
Copy link
Copy Markdown
Contributor

asheshv commented Apr 5, 2026

I am not against the idea, but - I have few questions and concerns.

Please find them as below:

  • Is this tool configurable at global configure?
  • How do I enable this for certain users only? (pgAdmin 4 has concept of roles).
  • Does this third party stores any information about the user or database schema post request? Does it follow any GDPR or other Data Protectection regulation?
  • I have a concern about the title 'Explain PostgreSQL'. I feel - it's misleading, it should have been 'Explian - tensor.ru'.

@dpage I think - this can be better implemented using plugin mechanism to avoid any compliance issue in core pgAdmin 4.

NOTE:
We don't have any plugin mechanism present at the moment. But - we shoud have some plugin mechanism to replace existing explain module with third party explain module during startup only.

@asheshv asheshv added Feature javascript Pull requests that update Javascript code labels Apr 5, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (2)
web/pgadmin/tools/expl_tensor/static/js/index.jsx (1)

20-25: Unused styled class definition.

The Explain-tabPanel class defined in StyledBox is never applied to any element in the component's render output. Consider removing this unused styling or applying it where intended.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/static/js/index.jsx` around lines 20 - 25,
StyledBox declares a nested CSS rule for the class 'Explain-tabPanel' that is
not used in the component; either remove that unused rule from StyledBox or
apply the class to the intended element. Locate the StyledBox styled component
in index.jsx and either delete the '& .Explain-tabPanel' block or add
className="Explain-tabPanel" (or the equivalent MUI class prop) to the element
representing the tab panel so the styling is actually applied.
web/pgadmin/tools/expl_tensor/__init__.py (1)

255-270: Non-standard "Method": "POST" header.

Line 260 includes "Method": "POST" as an HTTP header, which is non-standard. The HTTP method is already determined by the request itself (POST when data is provided). This header will be sent verbatim to the server and may cause confusion or be ignored. If this is intentionally required by the Explain Tensor API, please add a comment explaining why; otherwise, remove it.

♻️ Proposed fix
     headers = {
         "Content-Type": "application/json; charset=utf-8",
         "User-Agent": "pgAdmin4/ExplainTensor",
-        "Method": "POST"
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/__init__.py` around lines 255 - 270, The
headers dict in send_post_request contains a non-standard "Method": "POST" entry
which should be removed; update the headers in send_post_request to omit the
"Method" key (leave "Content-Type" and "User-Agent"), and if the API truly
requires a spoofed method, add a clear comment above send_post_request
explaining the requirement and the reason for including a custom header instead
of relying on urllib.request.Request's data-driven POST behavior; ensure no
other code (e.g., where no302opener is used) relies on that header.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/config.py`:
- Around line 1053-1061: The default enables a third-party Explain Tensor
integration system-wide via the EXPLAIN_TENSOR_ENABLED constant; change its
default to opt-in by setting EXPLAIN_TENSOR_ENABLED = False, update the
surrounding comment to state this integration is disabled by default for
privacy/security and must be explicitly enabled by an admin, and ensure any docs
or startup messages reference the new opt-in default so operators know to enable
it intentionally.

In `@web/pgadmin/static/js/components/ReactCodeMirror/index.jsx`:
- Around line 78-91: Replace the render-local explTensorEnabled boolean with
React state and fetch the status inside a useEffect so the GET
(api.get(url_for('expl_tensor.status'))) runs only when needed; create a state
hook (e.g., [explTensorEnabled, setExplTensorEnabled] = useState(false)), call
getApiInstance() inside the component or effect as before, then in the promise
setExplTensorEnabled(res.data?.data?.enabled === true) (use the endpoint's final
data.enabled field, not data.system_enabled) and handle errors in the catch;
ensure the effect dependency list includes any relevant preference object
(explTensorPrefs) if you want refetch on preference changes to avoid repeated
requests on every render.

In `@web/pgadmin/tools/expl_tensor/__init__.py`:
- Around line 144-155: When handling an error from send_post_request in the
Explain Tensor API block, avoid assuming the returned exception has a .code
attribute; instead extract a safe status/code via getattr (e.g., code =
getattr(data, 'code', getattr(data, 'status', None))) or set code = None and
include a stringified exception in errormsg; then pass that safe code value into
make_json_response (replace direct data.code use) so AttributeError cannot occur
while preserving useful error info.
- Around line 201-211: The error handling block that returns a JSON response
after send_post_request (using variables is_error and response_data) assumes
response_data has a .code attribute and can raise AttributeError; change the
response construction in the make_json_response call to safely access the code
via getattr(response_data, 'code', None) (and keep errormsg=str(response_data)
and existing fields like api_url/gettext unchanged) so missing .code won't raise
an exception.

In `@web/pgadmin/tools/expl_tensor/static/js/formatSQL.js`:
- Around line 34-44: The code currently constructs and throws an Error
containing btf_query_text and then console.error(err), leaking user SQL to the
browser console; change the thrown error to a generic message (e.g. "Remote
formatter failed") instead of including btf_query_text and update the catch to
log a non-sensitive, generic error message or rethrow without logging the raw
SQL. Locate the throw site that references btf_query_text and the surrounding
try/catch in formatSQL.js (the block that compares btf_query !== btf_query_text)
and replace the error payload and console.error usage so no raw query text is
logged or embedded in the Error.

In `@web/pgadmin/tools/expl_tensor/static/js/index.jsx`:
- Around line 37-80: Inside the useEffect callback (the function that runs when
plans change), convert the request flow so the inner fetch is awaited/returned
so the outer finally waits for it (e.g., make the effect callback async or
create an async helper and await api.post and, if needed, await
fetch(res.data.data.url)) to avoid the race that sets setIsLoading(false) before
inner fetch completes; also change the loose equality check on
res.data?.data?.code from == to ===; and clear any previous error at the start
of the request (call setError(null) before sending the request) so stale errors
don’t persist when new plans arrive — reference the useEffect callback, api.post
call, the inner fetch invocation, setIsLoading, setData, and setError when
making these changes.

In `@web/pgadmin/tools/expl_tensor/tests/test_format_sql.py`:
- Around line 37-65: The HTTP POST calls using self.tester.post (both the
success and failure branches) are executed after the with patch(...) block so
the mocks for get_preference_value, is_valid_url, and send_post_request are not
applied; move the entire request/response/assertion logic (the blocks that call
self.tester.post, parse response_data, and assert on response_data['success'])
inside the with patch(...) context so the patched functions
(get_preference_value, is_valid_url, send_post_request) take effect during the
request handling; ensure both the if self.expected_success and else branches
remain inside that same with block and still reference self.url and self.data as
before.

In `@web/pgadmin/translations/ru/LC_MESSAGES/messages.po`:
- Around line 16370-16374: The translation for msgid "Explain Tensor" currently
replaces the service name with a generic phrase; update the msgstr for msgid
"Explain Tensor" so it preserves the external service name "Tensor" (for
example: include "Tensor" verbatim or transliterated within the Russian string
such as "Explain Tensor — Анализ плана выполнения" or "Анализ плана выполнения
(Tensor)") to match the PR intent; locate the msgid "Explain Tensor" in the
translations file and modify its msgstr accordingly.
- Around line 16379-16382: The translation exposes an internal module token
("expl_tensor") in the user-facing msgstr; update the msgstr for the msgid
"Analyze query plan via Explain Tensor API" to remove the internal identifier
and use a concise, user-facing phrase (e.g. "Анализ плана запроса через Explain
Tensor API" or equivalent Russian wording without "expl_tensor") in the
messages.po entry corresponding to pgadmin/tools/expl_tensor/__init__.py:44.

---

Nitpick comments:
In `@web/pgadmin/tools/expl_tensor/__init__.py`:
- Around line 255-270: The headers dict in send_post_request contains a
non-standard "Method": "POST" entry which should be removed; update the headers
in send_post_request to omit the "Method" key (leave "Content-Type" and
"User-Agent"), and if the API truly requires a spoofed method, add a clear
comment above send_post_request explaining the requirement and the reason for
including a custom header instead of relying on urllib.request.Request's
data-driven POST behavior; ensure no other code (e.g., where no302opener is
used) relies on that header.

In `@web/pgadmin/tools/expl_tensor/static/js/index.jsx`:
- Around line 20-25: StyledBox declares a nested CSS rule for the class
'Explain-tabPanel' that is not used in the component; either remove that unused
rule from StyledBox or apply the class to the intended element. Locate the
StyledBox styled component in index.jsx and either delete the '&
.Explain-tabPanel' block or add className="Explain-tabPanel" (or the equivalent
MUI class prop) to the element representing the tab panel so the styling is
actually applied.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 424834d1-9c83-47c3-b257-ea77bfbdedb5

📥 Commits

Reviewing files that changed from the base of the PR and between e4f886a and 63a062c.

⛔ Files ignored due to path filters (10)
  • docs/en_US/images/expl_tensor_analyze.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_diagram.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_personal.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_piechart.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_plantree.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_recs.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_schema.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_stats.png is excluded by !**/*.png
  • docs/en_US/images/expl_tensor_tilemap.png is excluded by !**/*.png
  • docs/en_US/images/preferences_expl_tensor.png is excluded by !**/*.png
📒 Files selected for processing (20)
  • docs/en_US/developer_tools.rst
  • docs/en_US/expl_tensor.rst
  • docs/en_US/preferences.rst
  • web/config.py
  • web/pgadmin/messages.pot
  • web/pgadmin/static/js/components/ReactCodeMirror/index.jsx
  • web/pgadmin/tools/__init__.py
  • web/pgadmin/tools/expl_tensor/__init__.py
  • web/pgadmin/tools/expl_tensor/static/js/formatSQL.js
  • web/pgadmin/tools/expl_tensor/static/js/index.jsx
  • web/pgadmin/tools/expl_tensor/tests/__init__.py
  • web/pgadmin/tools/expl_tensor/tests/test_explain_functionality.py
  • web/pgadmin/tools/expl_tensor/tests/test_format_sql.py
  • web/pgadmin/tools/expl_tensor/tests/test_preferences.py
  • web/pgadmin/tools/expl_tensor/tests/test_status_endpoint.py
  • web/pgadmin/tools/expl_tensor/tests/test_url_validation.py
  • web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js
  • web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.mo
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.po
✅ Files skipped from review due to trivial changes (5)
  • web/pgadmin/tools/expl_tensor/tests/init.py
  • docs/en_US/developer_tools.rst
  • docs/en_US/preferences.rst
  • web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js
  • docs/en_US/expl_tensor.rst
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/pgadmin/tools/init.py
  • web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx

@MGorkov
Copy link
Copy Markdown
Author

MGorkov commented Apr 9, 2026

@asheshv Thank you for taking the time to review my PR and for your detailed feedback. I've addressed each of your points below and have made several updates to the code and documentation based on your suggestions.

  1. Is this tool configurable at global configure?

Yes. Based on your feedback, I've added a global configuration option. A new setting EXPLAIN_TENSOR_ENABLED is now available in the config file. This allows an administrator to completely disable the integration at the pgAdmin instance level. By default, Explain Tensor is disabled (EXPLAIN_TENSOR_ENABLED = False).

  1. How do I enable this for certain users only? (pgAdmin 4 has concept of roles)

To be honest, I'm not entirely sure I understand the point about database roles in this context. This feature replaces the EXPLAIN visualization in the pgAdmin UI — it's a frontend feature and isn't tied to PostgreSQL database roles.

  1. Does this third party store any information about the user or database schema post request? Does it follow any GDPR or other Data Protection regulation?

Currently, explain.tensor.ru does not have an explicit Privacy Policy — it will be added soon.

Regarding data storage: The service stores only the query text and the execution plan itself. No user data, IP addresses, database schemas, or any other metadata is collected or transmitted. The stored query and plan are not shared with any third parties.

By default, the stored query texts and execution plans are publicly accessible. However, the module provides a configuration option that allows users to hide plans from public access and store them in a personal archive instead. This setting can be enabled as needed.

  1. Concern about the title 'Explain PostgreSQL'

Fixed: The module has been renamed to "Explain Tensor" , and all documentation has been updated accordingly. The new name accurately reflects that this is an integration with an external explain.tensor.ru service, not a built-in PostgreSQL feature.

  1. Plugin mechanism (@dpage comment)

You make a very good point. A proper plugin mechanism would be the ideal solution to avoid any compliance issues in core pgAdmin. I fully support this idea.

Since no such plugin mechanism exists at the moment, I propose merging this PR as a temporary solution (with the global config flag disabled by default). Once the official plugin mechanism becomes available, the integration can be migrated to use it.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
web/pgadmin/tools/expl_tensor/tests/test_preferences.py (1)

10-14: Remove unused import.

unittest is imported but not used; only unittest.mock.patch and MagicMock are needed.

♻️ Proposed fix
 """Test the preferences functionality in Explain Tensor module."""
-import unittest
 from unittest.mock import patch, MagicMock
 from pgadmin.utils.route import BaseTestGenerator
 from pgadmin.tools.expl_tensor import MODULE_NAME
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/tests/test_preferences.py` around lines 10 -
14, Remove the unused top-level import of the unittest module in
test_preferences.py: delete the standalone "import unittest" and keep only the
used imports (from unittest.mock import patch, MagicMock) so there are no unused
imports; verify that no other code references the unittest symbol (e.g., in any
tests or helper functions) and run the tests to ensure imports still resolve for
BaseTestGenerator and MODULE_NAME.
web/pgadmin/tools/expl_tensor/__init__.py (3)

255-270: Consider adding request timeout documentation.

The 10-second timeout on line 264 is reasonable, but consider adding a comment explaining why this value was chosen, or making it configurable if longer queries/plans might need more time to process.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/__init__.py` around lines 255 - 270, The
send_post_request function hardcodes a 10-second timeout which should be
documented or made configurable; update send_post_request to accept an optional
timeout parameter (defaulting to 10) or read a module-level constant (e.g.,
EXPLAIN_TENSOR_REQUEST_TIMEOUT) and add a brief comment above the timeout usage
explaining why 10s was chosen and when callers should increase it for longer
plans, then use that variable when calling no302opener.open(req, timeout=...);
also update any call sites of send_post_request to pass a custom timeout where
longer processing is expected.

31-38: Conditional preference registration is non-standard for pgAdmin tools.

Unlike other tools (debugger, schema_diff) that unconditionally register preferences, this module gates registration on the config flag. This means if an admin later enables EXPLAIN_TENSOR_ENABLED, they must restart pgAdmin for preferences to appear.

This is acceptable given the privacy-sensitive nature of the integration, but consider documenting this restart requirement.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/__init__.py` around lines 31 - 38, The code
gates preference registration in register_preferences on
config.EXPLAIN_TENSOR_ENABLED which requires a pgAdmin restart to take effect;
update the codebase docs and/or add an inline comment in the
register_preferences function to explicitly state that preferences are only
registered at startup and enabling EXPLAIN_TENSOR_ENABLED at runtime will not
surface preferences until pgAdmin is restarted, and add a brief note to the
relevant admin/configuration documentation describing this restart requirement.

144-157: URL path concatenation may produce double slashes.

If explain_tensor_api ends with a trailing slash (e.g., https://explain.tensor.ru/), the concatenation produces https://explain.tensor.ru//beautifier-api. While most servers normalize this, it's cleaner to strip trailing slashes.

♻️ Proposed fix
-    api_url = explain_tensor_api + '/beautifier-api'
+    api_url = explain_tensor_api.rstrip('/') + '/beautifier-api'

Apply the same pattern at line 200:

-    api_url = explain_tensor_api + '/explain'
+    api_url = explain_tensor_api.rstrip('/') + '/explain'

And at line 220:

-    res_data = explain_tensor_api + response_data
+    res_data = explain_tensor_api.rstrip('/') + response_data
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/__init__.py` around lines 144 - 157, The URL
concatenation builds api_url via explain_tensor_api + '/beautifier-api' which
can produce double slashes if explain_tensor_api has a trailing slash; update
the construction to normalize the base by stripping trailing slashes (e.g., use
explain_tensor_api.rstrip('/') before appending) and apply the same
normalization pattern for the other URL constructions referenced around the
other occurrences (the similar builds at the locations you noted around line
~200 and ~220), ensuring send_post_request is called with the normalized
api_url.
web/pgadmin/tools/expl_tensor/tests/test_format_sql.py (1)

11-14: Remove unused import.

unittest is imported but not used directly; only unittest.mock.patch is needed.

♻️ Proposed fix
 """Test the SQL formatting functionality in Explain Tensor module."""
 import json
-import unittest
 from unittest.mock import patch
 from pgadmin.utils.route import BaseTestGenerator
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/expl_tensor/tests/test_format_sql.py` around lines 11 - 14,
Remove the unused top-level import "unittest" from the import block and keep
only the required symbols (e.g., import patch from unittest.mock); update the
imports so they read "import json\nfrom unittest.mock import patch\nfrom
pgadmin.utils.route import BaseTestGenerator" (or equivalent) to eliminate the
unused symbol while preserving json, patch, and BaseTestGenerator references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/config.py`:
- Around line 1053-1061: Fix the typo in the comment above the
EXPLAIN_TENSOR_ENABLED constant: change "Eplain Tensor" to "Explain Tensor" so
the comment reads "Explain Tensor features" and any other occurrences in that
comment block are corrected to maintain consistent spelling for the
EXPLAIN_TENSOR_ENABLED setting.

---

Nitpick comments:
In `@web/pgadmin/tools/expl_tensor/__init__.py`:
- Around line 255-270: The send_post_request function hardcodes a 10-second
timeout which should be documented or made configurable; update
send_post_request to accept an optional timeout parameter (defaulting to 10) or
read a module-level constant (e.g., EXPLAIN_TENSOR_REQUEST_TIMEOUT) and add a
brief comment above the timeout usage explaining why 10s was chosen and when
callers should increase it for longer plans, then use that variable when calling
no302opener.open(req, timeout=...); also update any call sites of
send_post_request to pass a custom timeout where longer processing is expected.
- Around line 31-38: The code gates preference registration in
register_preferences on config.EXPLAIN_TENSOR_ENABLED which requires a pgAdmin
restart to take effect; update the codebase docs and/or add an inline comment in
the register_preferences function to explicitly state that preferences are only
registered at startup and enabling EXPLAIN_TENSOR_ENABLED at runtime will not
surface preferences until pgAdmin is restarted, and add a brief note to the
relevant admin/configuration documentation describing this restart requirement.
- Around line 144-157: The URL concatenation builds api_url via
explain_tensor_api + '/beautifier-api' which can produce double slashes if
explain_tensor_api has a trailing slash; update the construction to normalize
the base by stripping trailing slashes (e.g., use explain_tensor_api.rstrip('/')
before appending) and apply the same normalization pattern for the other URL
constructions referenced around the other occurrences (the similar builds at the
locations you noted around line ~200 and ~220), ensuring send_post_request is
called with the normalized api_url.

In `@web/pgadmin/tools/expl_tensor/tests/test_format_sql.py`:
- Around line 11-14: Remove the unused top-level import "unittest" from the
import block and keep only the required symbols (e.g., import patch from
unittest.mock); update the imports so they read "import json\nfrom unittest.mock
import patch\nfrom pgadmin.utils.route import BaseTestGenerator" (or equivalent)
to eliminate the unused symbol while preserving json, patch, and
BaseTestGenerator references.

In `@web/pgadmin/tools/expl_tensor/tests/test_preferences.py`:
- Around line 10-14: Remove the unused top-level import of the unittest module
in test_preferences.py: delete the standalone "import unittest" and keep only
the used imports (from unittest.mock import patch, MagicMock) so there are no
unused imports; verify that no other code references the unittest symbol (e.g.,
in any tests or helper functions) and run the tests to ensure imports still
resolve for BaseTestGenerator and MODULE_NAME.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 32edf3a1-8069-45ef-97ad-6aefbfa6e5ca

📥 Commits

Reviewing files that changed from the base of the PR and between 63a062c and bf817c4.

📒 Files selected for processing (8)
  • web/config.py
  • web/pgadmin/messages.pot
  • web/pgadmin/tools/expl_tensor/__init__.py
  • web/pgadmin/tools/expl_tensor/static/js/index.jsx
  • web/pgadmin/tools/expl_tensor/tests/test_format_sql.py
  • web/pgadmin/tools/expl_tensor/tests/test_preferences.py
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.mo
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.po
🚧 Files skipped from review as they are similar to previous changes (3)
  • web/pgadmin/tools/expl_tensor/static/js/index.jsx
  • web/pgadmin/translations/ru/LC_MESSAGES/messages.po
  • web/pgadmin/messages.pot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants