This is an automated email from the ASF dual-hosted git repository.
voidmatcha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 00abf92f57 [MINOR] Interpreter completion candidates are filtered out
of the suggestion list
00abf92f57 is described below
commit 00abf92f578dac3447631c7bdc1d9f81c46bac07
Author: Changhoon Oh <[email protected]>
AuthorDate: Tue Sep 1 19:04:12 2026 +0900
[MINOR] Interpreter completion candidates are filtered out of the
suggestion list
### What is this PR for?
When an interpreter fills in the `meta` field of its completion candidates,
none of them reach the suggestion list. Pressing `Ctrl+.` shows nothing at all,
even though the backend answered with valid candidates.
`completionSupportWithBackend` decides what to keep by asking whether an
item carries a `meta` value:
```js
matches = matches.filter(function(item) {
if (!_.isEmpty(item.meta)) {
if (completionListLength !== 0) {
return false;
}
}
...
```
The intent of
[ZEPPELIN-3001](https://issues.apache.org/jira/browse/ZEPPELIN-3001)
(`fe07e5a49`) was to hide ace's own local/keyword suggestions once the
interpreter has answered, and `meta` was used as the marker for "this candidate
came from ace". That assumption does not hold — interpreters tag their own
candidates with `meta` as well. `SqlCompleter` sends `schema`, `table` and
`column`:
```java
addCompletions(candidates, schemaCandidates, CompletionType.schema.name());
addCompletions(candidates, columnCandidates, CompletionType.column.name());
addCompletions(candidates, tableCandidates, CompletionType.table.name());
```
So the filter drops the backend's own suggestions along with ace's, and the
list ends up empty.
The failure looks intermittent because the guard only lets anything through
while `completionListLength` happens to be `0`. That variable is declared per
paragraph controller but is updated through `$rootScope.$broadcast`, and it is
reset to `undefined` at the end of every filter pass — so whether anything
shows depends on what the other paragraphs in the note last answered.
Interpreters that leave `meta` empty are unaffected, which is why this went
unnoticed for so long.
This PR marks the candidates built by `remoteCompleter` with an explicit
`fromBackend` flag and filters on that instead of on `meta`. The original
intent is preserved — ace's own suggestions are still hidden once the
interpreter has answered — but the interpreter's candidates are always kept.
### What type of PR is it?
Bug Fix
### Todos
* [x] - Mark interpreter-provided candidates with an explicit `fromBackend`
flag
* [x] - Filter on that flag instead of on `meta`
### Related Jira issue (w/ regression) ?
* https://issues.apache.org/jira/browse/ZEPPELIN-3001
### How should this be tested?
No automated test is included: the change lives in the AngularJS notebook
UI (`zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js`), which
has no test harness covering ace's completion pipeline. Suggestions on how to
cover it are welcome.
Manual steps, using any interpreter that tags `meta` — the JDBC family does:
1. Bind the `jdbc` interpreter to a database that has at least one schema
with tables.
2. In a paragraph, type `SELECT * FROM <schema>.` and press `Ctrl+.`.
* **Before:** nothing appears. Occasionally the full ace list appears
instead, depending on what the other paragraphs in the note last answered.
* **After:** the schema's tables are listed with the `table` meta label,
consistently on every press, and ace's `local`/`keyword` entries are hidden as
ZEPPELIN-3001 intended.
3. Press `Ctrl+.` five or six times in a row and confirm the list is
identical every time.
4. Repeat in a paragraph bound to an interpreter that leaves `meta` empty
(for example `python`) and confirm its behavior is unchanged.
### Before & After
| | |
|---|---|
| **Before** | _(empty suggestion list on a JDBC paragraph after `Ctrl+.`)_
|
| **After** | _(the schema's tables listed with the `table` label)_ |
### Questions:
* Does the license files need to update? — No.
* Is there breaking changes for older versions? — No. The filter keeps its
original behavior for
interpreters that leave `meta` empty; only candidates that were
previously dropped by mistake now
appear.
* Does this needs documentation? — No.
Closes #5415 from okayhooni/hotfix/auto-completion.
Signed-off-by: YONGJAE LEE <[email protected]>
---
.../org/apache/zeppelin/jdbc/SqlCompleter.java | 3 +-
.../app/notebook/paragraph/paragraph.controller.js | 15 +-
.../paragraph/paragraph.controller.test.js | 159 +++++++++++++++++++++
3 files changed, 172 insertions(+), 5 deletions(-)
diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java
b/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java
index 20dc8d755d..8631e036f8 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java
@@ -333,7 +333,8 @@ public class SqlCompleter {
}
LOGGER.info("Completer initialized with " + schemas.size() + "
schemas, " +
- columns.size() + " tables and " + keywords.size() + " keywords");
+ tables.size() + " tables, " + columns.size() + " columns and " +
+ keywords.size() + " keywords");
}
} catch (SQLException | IOException e) {
diff --git a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js
b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js
index 69ee634af0..51acc75f5f 100644
--- a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js
+++ b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js
@@ -878,6 +878,12 @@ function ParagraphCtrl($scope, $rootScope, $route,
$window, $routeParams, $locat
meta: v.meta,
caption: computeCaption(v.name, v.meta),
score: 300,
+ // Marks this candidate as coming from the interpreter, so
that
+ // completionSupportWithBackend can tell it apart from
ace's own local/keyword
+ // completions. `meta` cannot serve that purpose:
interpreters are free to fill it
+ // in (SqlCompleter sends schema/table/column), and using
it as the discriminator
+ // made those very suggestions disappear.
+ fromBackend: true,
});
}
}
@@ -1084,10 +1090,11 @@ function ParagraphCtrl($scope, $rootScope, $route,
$window, $routeParams, $locat
let prev = null;
matches = matches.filter(function(item) {
- if (!_.isEmpty(item.meta)) {
- if (completionListLength !== 0) {
- return false;
- }
+ // ZEPPELIN-3001 intent: once the interpreter has answered, hide ace's
own local/keyword
+ // suggestions so the list only shows what the backend knows. Candidates
coming from the
+ // interpreter are always kept - they are the reason it was asked in the
first place.
+ if (!item.fromBackend && completionListLength !== 0) {
+ return false;
}
let caption = item.snippet || item.caption || item.value;
if (caption === prev) {
diff --git
a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.test.js
b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.test.js
index 38d5480e02..2948cfdf4a 100644
--- a/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.test.js
+++ b/zeppelin-web/src/app/notebook/paragraph/paragraph.controller.test.js
@@ -2,6 +2,7 @@ describe('Controller: ParagraphCtrl', function() {
beforeEach(angular.mock.module('zeppelinWebApp'));
let scope;
+ let rootScope;
let websocketMsgSrvMock = {};
let paragraphMock = {
config: {},
@@ -19,6 +20,7 @@ describe('Controller: ParagraphCtrl', function() {
beforeEach(inject(function($controller, $rootScope) {
scope = $rootScope.$new();
+ rootScope = $rootScope;
$rootScope.notebookScope = $rootScope.$new(true, $rootScope);
$controller('ParagraphCtrl', {
@@ -50,4 +52,161 @@ describe('Controller: ParagraphCtrl', function() {
it('should set default value of "paragraphFocused" as false', function() {
expect(scope.paragraphFocused).toEqual(false);
});
+
+ describe('completion candidate filtering', function() {
+ let FilteredList;
+ let originalSetFilter;
+
+ let completionParagraph = {
+ id: 'paragraph_completion',
+ config: {
+ editorSetting: {
+ completionSupport: true,
+ },
+ },
+ settings: {
+ forms: {},
+ },
+ };
+
+ let fromInterpreter = function(value, meta) {
+ return {value: value, caption: value, meta: meta, score: 300,
fromBackend: true};
+ };
+
+ let fromAce = function(value, meta) {
+ return {value: value, caption: value, meta: meta, score: 0};
+ };
+
+ // the filter lives on ace's FilteredList prototype, installed on focus
+ let applyFilter = function(candidates) {
+ let list = new FilteredList(candidates);
+ list.setFilter('');
+ return list.filtered;
+ };
+
+ beforeEach(function() {
+ FilteredList = ace.require('ace/autocomplete').FilteredList;
+ originalSetFilter = FilteredList.prototype.setFilter;
+ scope.init(completionParagraph);
+ rootScope.$broadcast('focusParagraph', completionParagraph.id, 0, 0,
true);
+ });
+
+ afterEach(function() {
+ FilteredList.prototype.setFilter = originalSetFilter;
+ });
+
+ it('should keep interpreter candidates that carry a meta label',
function() {
+ let table = fromInterpreter('my_table', 'table');
+ expect(applyFilter([table])).toContain(table);
+ });
+
+ it('should hide ace candidates while interpreter candidates are
available', function() {
+ let table = fromInterpreter('my_table', 'table');
+ let local = fromAce('my_local_var', 'local');
+ let keyword = fromAce('select', 'keyword');
+
+ let filtered = applyFilter([table, local, keyword]);
+
+ expect(filtered).toContain(table);
+ expect(filtered).not.toContain(local);
+ expect(filtered).not.toContain(keyword);
+ });
+
+ describe('when the interpreter answers with no candidates', function() {
+ let editorElement;
+
+ // completionListLength is only settable through a listener aceLoaded
registers
+ beforeEach(function() {
+ websocketMsgSrvMock.getEditorSetting = function() {};
+ websocketMsgSrvMock.completion = function() {};
+
+ editorElement = document.createElement('div');
+ editorElement.id = 'completion_test_editor';
+ document.body.appendChild(editorElement);
+
+ scope.aceLoaded(ace.edit(editorElement));
+ rootScope.$broadcast('completionListLength', 0);
+ });
+
+ afterEach(function() {
+ document.body.removeChild(editorElement);
+ });
+
+ it('should fall back to ace candidates', function() {
+ let local = fromAce('my_local_var', 'local');
+ let keyword = fromAce('select', 'keyword');
+
+ let filtered = applyFilter([local, keyword]);
+
+ expect(filtered).toContain(local);
+ expect(filtered).toContain(keyword);
+ });
+ });
+
+ describe('candidates built from an interpreter answer', function() {
+ let editorElement;
+ let editor;
+ let remoteCompleter;
+
+ beforeEach(function() {
+ websocketMsgSrvMock.getEditorSetting = function() {};
+ websocketMsgSrvMock.completion = function() {};
+
+ editorElement = document.createElement('div');
+ editorElement.id = 'completion_producer_editor';
+ document.body.appendChild(editorElement);
+
+ editor = ace.edit(editorElement);
+
+ scope.aceLoaded(editor);
+
+ // getCompletions returns without registering its listener unless the
editor is focused,
+ // and a headless run does not reliably grant it.
+ editor.isFocused = function() {
+ return true;
+ };
+
+ // Captured now, not at call time: setCompleters writes a
module-global list.
+ remoteCompleter = editor.completers[0];
+ });
+
+ afterEach(function() {
+ document.body.removeChild(editorElement);
+ });
+
+ let collectCandidates = function(completions) {
+ let received = null;
+ let called = false;
+
+ remoteCompleter.getCompletions(editor, editor.getSession(), {row: 0,
column: 0}, '', function(err, items) {
+ called = true;
+ received = items;
+ });
+ rootScope.$broadcast('completionList', {completions: completions});
+
+ // Otherwise a missing answer surfaces as a null-shape error somewhere
else.
+ expect(called).toBe(true);
+ return received;
+ };
+
+ it('should mark every candidate as coming from the interpreter',
function() {
+ let candidates = collectCandidates([
+ {name: 'my_table', value: 'my_table', meta: 'table'},
+ {name: 'my_schema', value: 'my_schema', meta: 'schema'},
+ ]);
+
+ expect(candidates).not.toBeNull();
+ expect(candidates.length).toEqual(2);
+ candidates.forEach(function(candidate) {
+ expect(candidate.fromBackend).toBe(true);
+ });
+ });
+
+ it('should produce candidates that survive the filter', function() {
+ let candidates = collectCandidates([{name: 'my_table', value:
'my_table', meta: 'table'}]);
+
+ expect(applyFilter(candidates)).toEqual(candidates);
+ });
+ });
+ });
});
