bamaer commented on code in PR #8784:
URL: https://github.com/apache/hop/pull/8784#discussion_r4210614772
##########
plugins/tech/ai/src/main/java/org/apache/hop/ai/engine/AiAdvisorEngine.java:
##########
@@ -128,7 +483,10 @@ static AiAdvisorRequest toRequest(
request.setAttributes(copyAttributes(session.getAttributes()));
request.setInclusionSelections(copyInclusionSelections(session.getInclusionSelections()));
request.setFollowUp(hasSuccessfulPriorTurn(session));
-
request.setAppliedChangeSummaries(session.consumePendingAppliedSummaries());
+ request.setAppliedChangeSummaries(
+ consumeApplied
+ ? session.consumePendingAppliedSummaries()
Review Comment:
Fixed. The request now gets a copy of the applied changes. They are taken
off the session only when an answer is recorded, and only the ones that
question was sent with, so changes applied while it waits stay for the next
question.
##########
plugins/tech/ai/src/main/java/org/apache/hop/ai/ui/AiAdvisorSessionPane.java:
##########
@@ -947,58 +1501,101 @@ private void send() {
}
HopAiConfig config = HopAiConfigSingleton.getConfig();
if (!config.isAiEnabled()) {
- wlStatus.setText(BaseMessages.getString(PKG,
"AiAdvisor.Status.Disabled"));
+ setStatus(BaseMessages.getString(PKG, "AiAdvisor.Status.Disabled"));
return;
}
String prompt = wPrompt.getText();
- if (Utils.isEmpty(prompt)) {
+ if (Utils.isEmpty(prompt) || prompt.isBlank()) {
+ setStatus(BaseMessages.getString(PKG, "AiAdvisor.Status.NoQuestion"));
+ wPrompt.setFocus();
return;
}
IAiAdvisor advisor = loadSelectedAdvisor();
if (advisor == null) {
- wlStatus.setText(BaseMessages.getString(PKG,
"AiAdvisor.Status.NoAdvisor"));
+ setStatus(noAdvisorMessage());
+ return;
+ }
+ if (!providerExists(wProvider.getText())) {
+ setStatus(
+ BaseMessages.getString(
+ PKG,
+ wProvider.getItemCount() == 0
+ ? "AiAdvisor.Status.NoProviderDefined"
+ : "AiAdvisor.Status.NoProviderSelected"));
return;
}
session.setAdvisorPluginId(selectedAdvisorId());
session.setScenarioId(selectedScenarioId());
session.setProviderName(wProvider.getText());
+ includeLogAfterARun();
for (Map.Entry<String, Button> entry : inclusionButtons.entrySet()) {
- session.getInclusions().put(entry.getKey(),
entry.getValue().getSelection());
+ // A blocked option keeps the user's choice for when the configuration
allows it again.
+ if (!isBlockedByConfig(entry.getKey())) {
+ session.getInclusions().put(entry.getKey(),
entry.getValue().getSelection());
+ }
}
AiAdvisorTurn turn = new AiAdvisorTurn();
turn.setUserPrompt(prompt);
+ turn.setStartedAtMillis(System.currentTimeMillis());
session.addTurn(turn);
session.setCancelled(false);
session.setWorking(true);
- session.setStatusMessage(BaseMessages.getString(PKG,
"AiAdvisor.Status.Working"));
+ // The transcript shows a live waiting line; the status line is for
problems.
+ session.setStatusMessage("");
wPrompt.setText("");
updateSendButton();
- wlStatus.setText(session.getStatusMessage());
+ setStatus(session.getStatusMessage());
transcript.showSession(session, this::reviewProposalsForTurn);
store.fireChanged();
AiAdvisorSession target = session;
- // Log widgets are SWT; read them on this UI thread before the background
worker starts.
- final String logExcerpt =
- target.getLogSupplier() != null ? target.getLogSupplier().get() : null;
+ sentFromHere.add(target);
+ // Build the question here on the UI thread: the log widgets are SWT, and
the pipeline or
+ // workflow may be edited while the model works. Only the model call runs
in the background.
+ AiAdvisorEngine.Prepared prepared;
+ try {
+ String logExcerpt = target.getLogSupplier() != null ?
target.getLogSupplier().get() : null;
+ prepared =
+ AiAdvisorEngine.prepare(
+ target, advisor, host.getVariables(),
host.getMetadataProvider(), logExcerpt);
+ } catch (Exception e) {
+ completeTurn(target, turn, null, e);
+ return;
+ }
+ turn.setEstimatedPromptTokens(prepared.estimatedTokens());
+ turn.setProviderLabel(prepared.provider().getName());
+ startWorkingLine(target);
+ IVariables variables = host.getVariables();
+ Display display = getDisplay();
Thread worker =
BackgroundThreadFacade.start(
() -> {
+ AiAdvisorResponse response = null;
+ Throwable error = null;
try {
- AiAdvisorResponse response =
- AiAdvisorEngine.advise(
- target,
- advisor,
- host.getVariables(),
- host.getMetadataProvider(),
- logExcerpt);
- host.asyncExec(() -> completeTurn(target, turn, response,
null));
+ response = AiAdvisorEngine.execute(target, advisor, variables,
prepared);
} catch (Throwable t) {
if (t instanceof InterruptedException) {
Thread.currentThread().interrupt();
}
- host.asyncExec(() -> completeTurn(target, turn, null, t));
+ error = t;
+ }
+ AiAdvisorResponse result = response;
+ Throwable failure = error;
+ Thread self = Thread.currentThread();
+ // Record the answer even when this window was closed in the
meantime (unless the
+ // configuration cancels it on close): the session is shared
with the other views
+ // and would otherwise stay "working" for good.
+ if (!display.isDisposed()) {
Review Comment:
Agreed the option promised too much, but recording from the worker can't
help here: `HopGui.main` calls `System.exit` as soon as `open()` returns, so
the worker is killed. I dropped "or Hop GUI" from the tooltip and docs, which
now say exiting always stops a waiting question. The session store marks that
question as stopped before the final save on exit, so after a restart it says
so instead of sitting unanswered.
##########
plugins/tech/ai/src/main/java/org/apache/hop/ai/advisors/pipeline/PipelineAiProposalValidator.java:
##########
@@ -177,43 +222,75 @@ private static AiProposalValidation
validateAddPipelineHop(
String fromName = proposal.parameter("fromTransform");
String toName = proposal.parameter("toTransform");
if (Utils.isEmpty(fromName) || Utils.isEmpty(toName)) {
- return blocked(proposal, "fromTransform and toTransform are required");
+ return blocked(
+ proposal, BaseMessages.getString(PKG,
"PipelineAiProposalValidator.HopEndsRequired"));
}
if (!transformExists(pipelineMeta, fromName, reservedNames)) {
- return blocked(proposal, "From transform not found: " + fromName);
+ return blocked(
+ proposal,
+ BaseMessages.getString(
+ PKG, "PipelineAiProposalValidator.FromTransformNotFound",
fromName));
}
if (!transformExists(pipelineMeta, toName, reservedNames)) {
- return blocked(proposal, "To transform not found: " + toName);
+ return blocked(
+ proposal,
+ BaseMessages.getString(PKG,
"PipelineAiProposalValidator.ToTransformNotFound", toName));
}
TransformMeta from = pipelineMeta.findTransform(fromName);
TransformMeta to = pipelineMeta.findTransform(toName);
if (fromName.trim().equals(toName.trim())) {
- return blocked(proposal, "Hop cannot connect a transform to itself");
- }
+ return blocked(
+ proposal, BaseMessages.getString(PKG,
"PipelineAiProposalValidator.HopToItself"));
+ }
+ // A pipeline cannot loop: the reverse hop, already there or proposed
earlier, would. Not when
+ // the batch deletes it.
+ if ((from != null
Review Comment:
Fixed. The validator follows the hops as they will be once the batch is
applied: existing hops minus the deleted ones, with the batch's renames and
transform deletes, plus the hops added earlier in the batch. It blocks a loop
of any length and names the path in the message, for example `C -> A -> B ->
C`. Names are trimmed. Tests cover a loop through three transforms and one
through a renamed transform.
##########
plugins/tech/ai/src/main/java/org/apache/hop/ai/engine/AiNodeSettings.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hop.ai.engine;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.core.util.Utils;
+import org.apache.hop.metadata.api.HopMetadataProperty;
+import org.apache.hop.metadata.util.ReflectionUtil;
+
+/**
+ * The settings of a transform or action, as compact JSON for a prompt.
+ *
+ * <p>Reads the {@code @HopMetadataProperty} fields: SQL, file names,
connection names, field lists
+ * and so on, which is what a model needs to explain what a pipeline or
workflow does. Password
+ * fields are skipped. Empty values, long texts and long lists are left out or
shortened so one
+ * transform cannot take over the prompt.
+ */
+public final class AiNodeSettings {
+
+ static final int MAX_DEPTH = 3;
+ static final int MAX_LIST_ITEMS = 25;
+ static final int MAX_TEXT_CHARS = 400;
+ static final int MAX_NODE_CHARS = 3_000;
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ private AiNodeSettings() {}
+
+ /**
+ * @param node the transform or action meta
+ * @return a JSON object, {@code {"truncated":true}} when the settings are
too large, or null when
+ * the plugin exposes no settings this way
+ */
+ public static String toJson(Object node) {
+ if (node == null) {
+ return null;
+ }
+ Object settings = read(node, 0);
+ if (!(settings instanceof Map<?, ?> map) || map.isEmpty()) {
+ return null;
+ }
+ try {
+ String json = MAPPER.writeValueAsString(settings);
+ if (json.length() > MAX_NODE_CHARS) {
+ return "{\"truncated\":true}";
+ }
+ return AiTextUtil.redactSecrets(json);
+ } catch (JsonProcessingException e) {
+ return null;
+ }
+ }
+
+ static Object read(Object value, int depth) {
+ if (value == null) {
+ return null;
+ }
+ if (value instanceof String text) {
+ if (text.isEmpty()) {
+ return null;
+ }
+ return text.length() > MAX_TEXT_CHARS ? text.substring(0,
MAX_TEXT_CHARS) + "…" : text;
+ }
+ if (value instanceof Number || value instanceof Boolean) {
+ return value;
+ }
+ if (value instanceof Enum<?> e) {
+ return e.name();
+ }
+ if (depth >= MAX_DEPTH) {
+ return null;
+ }
+ if (value instanceof Collection<?> collection) {
+ return readList(collection, depth);
+ }
+ if (value.getClass().isArray() && value instanceof Object[] array) {
+ return readList(List.of(array), depth);
+ }
+ return readObject(value, depth);
+ }
+
+ private static List<Object> readList(Collection<?> collection, int depth) {
+ List<Object> list = new ArrayList<>();
+ int count = 0;
+ for (Object item : collection) {
+ if (count == MAX_LIST_ITEMS) {
+ list.add("… " + (collection.size() - MAX_LIST_ITEMS) + " more");
+ break;
+ }
+ Object read = read(item, depth + 1);
+ if (read != null) {
+ list.add(read);
+ }
+ count++;
+ }
+ return list.isEmpty() ? null : list;
+ }
+
+ private static Map<String, Object> readObject(Object object, int depth) {
+ Map<String, Object> map = new LinkedHashMap<>();
+ for (Field field : ReflectionUtil.findAllFields(object.getClass())) {
+ HopMetadataProperty property =
field.getAnnotation(HopMetadataProperty.class);
+ if (property == null || property.password()) {
+ continue;
+ }
+ Object fieldValue;
+ try {
+ fieldValue =
+ ReflectionUtil.getFieldValue(object, field.getName(),
isBoolean(field.getType()));
+ } catch (Exception e) {
+ continue;
+ }
+ Object read = read(fieldValue, depth + 1);
+ if (read == null || Boolean.FALSE.equals(read)) {
Review Comment:
Fixed. `false` is kept; only null and empty values are skipped.
##########
plugins/tech/ai/src/main/java/org/apache/hop/ai/session/AiAdvisorSessionArchive.java:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hop.ai.session;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.ai.advisor.AiAdvisorMetadataSelection;
+import org.apache.hop.ai.advisor.AiProposal;
+import org.apache.hop.ai.engine.AiMetadataBackup;
+import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.util.Utils;
+import org.apache.hop.history.AuditManager;
+import org.apache.hop.history.AuditState;
+
+/**
+ * Keeps the AI Assistant sessions of a project between Hop GUI runs, in the
audit folder ({@code
+ * HOP_AUDIT_FOLDER}), where Hop GUI also remembers the open files of each
project.
+ *
+ * <p>Questions, answers, proposals and the session options are kept. The
context that was sent with
+ * a question is not: it is gathered again from the open file. The pipeline or
workflow itself is
+ * found again by its file name when it is opened.
+ */
+public final class AiAdvisorSessionArchive {
+
+ static final String AUDIT_TYPE = "ai-assistant";
+ static final String AUDIT_NAME = "sessions";
+ static final String DEFAULT_GROUP = "hop-gui";
+
+ /** The newest sessions per project that are kept; older ones are dropped
when saving. */
+ static final int MAX_SESSIONS = 50;
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ private AiAdvisorSessionArchive() {}
+
+ public static void save(String scope, List<AiAdvisorSession> sessions)
throws HopException {
+ List<AiAdvisorSession> kept =
+ sessions.size() > MAX_SESSIONS
+ ? sessions.subList(sessions.size() - MAX_SESSIONS, sessions.size())
+ : sessions;
+ List<Map<String, Object>> list = new ArrayList<>();
+ for (AiAdvisorSession session : kept) {
+ list.add(toMap(session));
+ }
+ Map<String, Object> state = new LinkedHashMap<>();
+ try {
+ state.put("json", MAPPER.writeValueAsString(list));
+ } catch (Exception e) {
+ throw new HopException("Unable to write the AI Assistant sessions", e);
+ }
+ AuditManager.getActive()
+ .storeState(group(scope), AUDIT_TYPE, new AuditState(AUDIT_NAME,
state));
+ }
+
+ public static List<AiAdvisorSession> load(String scope) throws HopException {
+ List<AiAdvisorSession> sessions = new ArrayList<>();
+ AuditState state = AuditManager.getActive().retrieveState(group(scope),
AUDIT_TYPE, AUDIT_NAME);
+ if (state == null || state.getStateMap() == null) {
+ return sessions;
+ }
+ Object json = state.getStateMap().get("json");
+ if (!(json instanceof String text) || Utils.isEmpty(text)) {
+ return sessions;
+ }
+ try {
+ List<Map<String, Object>> list =
+ MAPPER.readValue(text, new TypeReference<List<Map<String,
Object>>>() {});
+ for (Map<String, Object> map : list) {
+ AiAdvisorSession session = fromMap(map);
+ session.setScope(scope);
+ sessions.add(session);
+ }
+ } catch (Exception e) {
+ throw new HopException("Unable to read the saved AI Assistant sessions",
e);
+ }
+ return sessions;
+ }
+
+ static final String LAST_PROVIDER_NAME = "last-provider";
+
+ /**
+ * Remember the provider picked last in a project, for new sessions when no
default provider is
+ * configured. Kept even when conversations are not: it holds only a name.
+ */
+ public static void saveLastProvider(String scope, String providerName)
throws HopException {
+ Map<String, Object> state = new LinkedHashMap<>();
+ state.put("name", providerName == null ? "" : providerName);
+ AuditManager.getActive()
+ .storeState(group(scope), AUDIT_TYPE, new
AuditState(LAST_PROVIDER_NAME, state));
+ }
+
+ /** The provider picked last in a project, or null when none was remembered.
*/
+ public static String loadLastProvider(String scope) throws HopException {
+ AuditState state =
+ AuditManager.getActive().retrieveState(group(scope), AUDIT_TYPE,
LAST_PROVIDER_NAME);
+ if (state == null
+ || state.getStateMap() == null
+ || !(state.getStateMap().get("name") instanceof String name)
+ || Utils.isEmpty(name)) {
+ return null;
+ }
+ return name;
+ }
+
+ static String group(String scope) {
+ return Utils.isEmpty(scope) ? DEFAULT_GROUP : scope;
+ }
+
+ static Map<String, Object> toMap(AiAdvisorSession session) {
+ Map<String, Object> map = new LinkedHashMap<>();
+ map.put("title", session.getTitle());
+ map.put("advisorPluginId", session.getAdvisorPluginId());
+ map.put("scenarioId", session.getScenarioId());
+ map.put("providerName", session.getProviderName());
+ map.put("location", session.getLocation());
+ map.put("areaLabel", session.getAreaLabel());
+ map.put("artifactName", session.getArtifactName());
+ map.put("artifactKind", session.getArtifactKind());
+ map.put("artifactFilename", session.getArtifactFilename());
+ map.put("focusNodeName", session.getFocusNodeName());
+ map.put("inclusions", new LinkedHashMap<>(session.getInclusions()));
+ map.put("userChosenInclusions", new
ArrayList<>(session.getUserChosenInclusions()));
+ List<Map<String, String>> selections = new ArrayList<>();
+ for (AiAdvisorMetadataSelection selection :
session.getMetadataSelections()) {
+ Map<String, String> entry = new LinkedHashMap<>();
+ entry.put("typeKey", selection.getTypeKey());
+ entry.put("name", selection.getName());
+ selections.add(entry);
+ }
+ map.put("metadataSelections", selections);
+ map.put("inclusionSelections", new
LinkedHashMap<>(session.getInclusionSelections()));
+ List<Map<String, Object>> turns = new ArrayList<>();
+ for (AiAdvisorTurn turn : session.getTurns()) {
+ turns.add(toMap(turn));
+ }
+ map.put("turns", turns);
+ return map;
Review Comment:
Fixed. The pending list is saved with the session and restored on load, and
the next question uses it in the same way.
##########
plugins/tech/ai/src/main/resources/org/apache/hop/ai/config/messages/messages_en_US.properties:
##########
@@ -17,15 +17,22 @@
HopAiConfig.Tab.Name=AI Assistant
HopAiConfigOptionPlugin.GitWarning.Label=Use Git (or similar version control)
and create a feature branch before applying AI-driven changes.\nYou can then
revert the whole project, or selected files, if things do not go as intended.
HopAiConfigOptionPlugin.GitWarning.Tooltip=Graph Undo only reverses some
canvas edits. Metadata saves and pasted transforms or actions are not undone
that way. A feature branch is the reliable rollback.
-HopAiConfigOptionPlugin.AiEnabled.Label=Enable AI advisory
-HopAiConfigOptionPlugin.AiEnabled.Tooltip=When disabled, AI Help is hidden and
no prompts are sent. Default is off. Before applying AI changes, work on a Git
feature branch so you can revert the project if needed.
+HopAiConfigOptionPlugin.AiEnabled.Label=Enable the AI Assistant
+HopAiConfigOptionPlugin.AiEnabled.Tooltip=When disabled, the AI Assistant is
hidden and no prompts are sent. Default is off. Before applying AI changes,
work on a Git feature branch so you can revert the project if needed.
HopAiConfigOptionPlugin.DefaultProvider.Label=Default AI provider
-HopAiConfigOptionPlugin.DefaultProvider.Tooltip=Named AI Provider metadata
used by GUI advisors. Pipelines can still pick a different provider.
+HopAiConfigOptionPlugin.DefaultProvider.Tooltip=Named AI Provider metadata
used by the AI Assistant. Pipelines can still pick a different provider.
HopAiConfigOptionPlugin.AllowFullXml.Label=Allow sending full XML
HopAiConfigOptionPlugin.AllowFullXml.Tooltip=Second gate for the session
"Include full XML" checkbox. Off by default. Full XML is large and can contain
SQL, filenames and notes. Leave this off unless you need it, and only after
secrets live in environment variables or a keystore.
HopAiConfigOptionPlugin.ExtraContext.Label=Extra context
HopAiConfigOptionPlugin.ExtraContext.Tooltip=Standing notes sent with every AI
Assistant question. The default reminds the model that metadata names are
case-sensitive (`local` is not `Local`), that pipeline and workflow run
configurations are different types, and that Hop is not Kettle. Add project
conventions here.
HopAiConfigOptionPlugin.ExtraContextFiles.Label=Context files
-HopAiConfigOptionPlugin.ExtraContextFiles.Tooltip=One VFS path per line. Files
that exist are appended to the system prompt; missing files are skipped.
Default is '${PROJECT_HOME}/AGENTS.md'. Variables are resolved.
+HopAiConfigOptionPlugin.ExtraContextFiles.Tooltip=One VFS path per line. Files
that exist are appended to the system prompt as instructions the assistant
follows; missing files are skipped. Variables are resolved. Empty by default.
Only list files you wrote for the AI Assistant, for example
'${PROJECT_HOME}/ai-assistant.md': an AGENTS.md is usually written for coding
agents, and a file from someone else's project can steer the assistant.
Review Comment:
Fixed, along with the generated config docs partials.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
