diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 00000000..4e64a654 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,77 @@ +# Midscene-Java Codebase Review + +Данный документ представляет собой обзор текущего состояния репозитория `midscene-java` с фокусом на `midscene-core` и `midscene-web`. В обзоре рассматриваются узкие места в производительности, вопросы стабильности, а также проводится сравнение с родительским проектом на JavaScript (`midscene-js`) для выявления отсутствующего функционала, который можно портировать. + +--- + +## 1. Производительность (Performance) + +### 1.1 Ожидание загрузки страницы (Waiting Mechanisms) +- **Проблема**: В `SeleniumDriver` и `PlaywrightDriver` метод `waitUntilPageLoaded()` вызывается перед **каждым** действием (клик, ввод, скролл). +- **Детали реализации**: Метод использует `WaitingUtils.waitUntilWithoutException(2, 2000, ...)` (в Selenium), что означает опрос `document.readyState` с интервалом в 2 секунды (2000ms), при общем таймауте в 2 секунды. Это значит, что он либо ждет 2 секунды, либо сразу продолжает, проглатывая исключения. В Playwright используется интервал 200ms. +- **Влияние**: Это создает огромные задержки (overhead) для простых действий. Кроме того, Playwright уже имеет встроенные умные ожидания (auto-waiting). Переписывание их с помощью `page.evaluate("document.readyState")` ухудшает производительность. +- **Решение**: Использовать встроенные механизмы (например, `page.waitForLoadState()` в Playwright, `WebDriverWait` в Selenium). + +### 1.2 Синхронный File I/O в кэше (`TaskCache.java`) +- **Проблема**: При включенном кэшировании в файл (что по умолчанию в конфигурации), методы `appendToFile` и `saveToFile` выполняются синхронно внутри блока `synchronized (fileLock)` в том же потоке, где выполняется вызов AI. +- **Влияние**: Замедляет поток выполнения, особенно при больших дампах истории. +- **Решение**: Использовать асинхронную запись (например, `ExecutorService` с daemon-потоками), чтобы вынести I/O операции в фон. + +### 1.3 Повторные скриншоты (Screenshot capturing) +- **Проблема**: В `Orchestrator` скриншоты (`driver.getScreenshotBase64()`) снимаются как "до", так и "после" выполнения инструкции, а также для каждого `query`. На длинных скриптах с множественными assertions это приводит к большим задержкам из-за сериализации в Base64. +- **Решение**: Оптимизировать частоту снятия скриншотов. В JS-версии есть концепция "заморозки" контекста (см. раздел Features). + +### 1.4 Свайпы (Swipes) через `Thread.sleep` +- **Проблема**: Свайпы реализованы через цикл с `Thread.sleep(50)`. Это блокирует основной поток выполнения. + +--- + +## 2. Стабильность (Stability) + +### 2.1 Обработка ошибок в `aiWaitFor` +- **Проблема**: В `Agent.java` метод `aiWaitFor` пытается заинжектить скрипт `MutationObserver`. Если это не удается (например, страница перезагружается или строгие политики CSP), он откатывается на цикл с `Thread.sleep()`. +- **Влияние**: Это может привести к нестабильным тестам (flaky tests), так как AI будет постоянно опрашиваться на медленном fallback'е. + +### 2.2 Неэффективное перепланирование в `Orchestrator` +- **Проблема**: Если происходит ошибка, `Orchestrator.execute` очищает историю (`history.clear()`) и инвалидирует кэш на первой попытке, но не использует контекст ошибки (не отправляет AI сообщение с пояснением, *почему* действие упало, в структурированном виде, как это делает JS-версия с "reflection"). + +### 2.3 Многопоточность в `Context.java` +- **Проблема**: Список событий `events` обернут в `Collections.synchronizedList`, что защищает от простых гонок данных, но не спасает от `ConcurrentModificationException` при итерации по этому списку во время генерации отчета, если в это время туда пишутся новые логи (например, из фоновых `aiActionAsync`). +- **Решение**: Заменить на `CopyOnWriteArrayList` или явно синхронизировать блоки итерации. + +### 2.4 Формирование промптов (`PromptManager.java`) +- **Проблема**: String formatting больших JSON'ов может привести к ошибкам экранирования (escaping), если в тексте страницы или инструкции пользователя есть спецсимволы. + +--- + +## 3. Сравнение с `midscene-js` и функционал к портированию + +Проведя анализ `midscene-js` (JavaScript версии), я выявил следующие фичи, которых сейчас не хватает в Java, и которые можно реализовать: + +1. **`freezePageContext()` / `unfreezePageContext()`** + - В JS-версии можно заморозить контекст страницы, чтобы серия `aiQuery` или `aiAssert` использовала один и тот же скриншот. Это кардинально ускоряет массовое извлечение данных. В Java этого сейчас нет. + +2. **`deepLocate` (ранее `deepThink`)** + - В JS есть опция `deepLocate: true`, которая заставляет Midscene делать два прохода (two-pass) через AI для более точного позиционирования элементов. В Java есть поле `deepThink` в `LocateOptions`, но механизм двух проходов в `Planner` не реализован так же глубоко. + +3. **Prompting with Images (Изображения в промптах)** + - В `midscene-js` в методы `aiTap`, `aiAssert` можно передавать как текстовый промпт, так и массив дополнительных изображений (референсов), чтобы сказать "найди вот такую иконку [картинка]". Java сейчас поддерживает только текст. + +4. **Отмена выполнения (AbortSignal / Timeouts)** + - В JS все методы поддерживают `abortSignal` для прерывания долгих AI-запросов по таймауту. В Java методы (`aiActionAsync`) возвращают `CompletableFuture`, но их отмена не пробрасывается в Langchain4j модель, что приводит к зависшим соединениям. + +5. **`ReportMergingTool`** + - JS-библиотека позволяет объединять отчеты от разных тест-кейсов в один большой HTML-файл. На данный момент `midscene-visualizer` в Java генерирует отдельный HTML для каждого прогона. + +6. **Алиасы API и совместимость** + - В JS основным методом стал `aiAct()` / `ai()`, а старый `aiAction()` оставлен для совместимости. Java использует `aiAction()`. + - В JS для клика используется `aiTap()`, который более универсален для мобильных/веб платформ. + +## Итог и рекомендации + +1. **Исправить задержки в драйверах**: Убрать или переписать polling `document.readyState` в Web Drivers. Использовать нативные механизмы ожидания библиотек (Selenium/Playwright). +2. **Оптимизировать кэш**: Сделать сохранение в файл асинхронным, чтобы не тормозить основной поток. +3. **Реализовать фичи из JS**: + - `freezePageContext()` для значительного ускорения групповых запросов. + - Поддержку картинок в пользовательских промптах (`Prompting with images`). + - Объединение отчетов (Report Merging). diff --git a/midscene-core/src/main/java/com/midscene/core/agent/Agent.java b/midscene-core/src/main/java/com/midscene/core/agent/Agent.java index 017230eb..9bf29d22 100644 --- a/midscene-core/src/main/java/com/midscene/core/agent/Agent.java +++ b/midscene-core/src/main/java/com/midscene/core/agent/Agent.java @@ -15,8 +15,15 @@ import com.midscene.core.pojo.options.LocateOptions; import com.midscene.core.pojo.options.ScrollOptions; import com.midscene.core.pojo.options.WaitOptions; +import com.midscene.core.utils.WaitingUtils; +import java.util.concurrent.atomic.AtomicBoolean; import com.midscene.core.service.PageDriver; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; import lombok.extern.log4j.Log4j2; /** @@ -45,6 +52,16 @@ public class Agent { private final Orchestrator orchestrator; private final PageDriver driver; private TaskCache cache; + // Custom executor for async actions to allow proper thread interruption + private final ExecutorService asyncExecutor = Executors.newCachedThreadPool(new ThreadFactory() { + private final AtomicInteger counter = new AtomicInteger(1); + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r, "Agent-Async-" + counter.getAndIncrement()); + t.setDaemon(true); + return t; + } + }); public Agent(PageDriver driver, AIModel aiModel) { this(driver, aiModel, TaskCache.disabled(), 3); @@ -371,7 +388,6 @@ public void aiAssert(String assertion) { public void aiWaitFor(String assertion, WaitOptions options) { long timeoutMs = options.getTimeoutMs(); long checkIntervalMs = options.getCheckIntervalMs(); - long startTime = System.currentTimeMillis(); // Initialize MutationObserver try { @@ -380,7 +396,9 @@ public void aiWaitFor(String assertion, WaitOptions options) { log.warn("Failed to inject MutationObserver, falling back to polling: {}", e.getMessage()); } - while (System.currentTimeMillis() - startTime < timeoutMs) { + AtomicBoolean conditionSatisfied = new AtomicBoolean(false); + long timeoutSecs = (long) Math.ceil(timeoutMs / 1000.0); + WaitingUtils.waitUntilWithoutException(timeoutSecs, checkIntervalMs, () -> { boolean shouldCheck = true; try { Object result = driver.executeScript(CHECK_AND_RESET_MUTATION_SCRIPT); @@ -388,7 +406,6 @@ public void aiWaitFor(String assertion, WaitOptions options) { shouldCheck = (Boolean) result; } } catch (Exception e) { - // Fallback to true if script fails shouldCheck = true; } @@ -396,22 +413,20 @@ public void aiWaitFor(String assertion, WaitOptions options) { boolean result = aiBoolean("Is the following currently true? " + assertion); if (result) { log.info("Wait condition satisfied: {}", assertion); - return; + conditionSatisfied.set(true); + return true; } } - - try { - Thread.sleep(checkIntervalMs); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new RuntimeException("Wait interrupted", e); + return false; + }, "Wait for AI condition: " + assertion); + + if (!conditionSatisfied.get()) { + if (options.isThrowOnTimeout()) { + throw new RuntimeException("Wait timeout: " + assertion); + } else { + log.warn("Wait timed out: {}", assertion); } } - - if (options.isThrowOnTimeout()) { - throw new RuntimeException("Wait timeout: " + assertion); - } - log.warn("Wait timed out: {}", assertion); } /** @@ -432,7 +447,30 @@ public void aiWaitFor(String assertion) { * @return A CompletableFuture representing the action execution */ public CompletableFuture aiActionAsync(String instruction) { - return CompletableFuture.runAsync(() -> aiAction(instruction)); + CompletableFuture future = new CompletableFuture<>(); + + // Using ExecutorService to get a Future that can be properly interrupted on cancel + Future task = asyncExecutor.submit(() -> { + try { + aiAction(instruction); + if (!future.isCancelled()) { + future.complete(null); + } + } catch (Throwable t) { + if (!future.isCancelled()) { + future.completeExceptionally(t); + } + } + }); + + // When the CompletableFuture gets cancelled, interrupt the executing task + future.whenComplete((res, ex) -> { + if (future.isCancelled()) { + task.cancel(true); + } + }); + + return future; } // ========== Utility Methods ========== diff --git a/midscene-core/src/main/java/com/midscene/core/agent/Orchestrator.java b/midscene-core/src/main/java/com/midscene/core/agent/Orchestrator.java index e7603c88..697f6f3b 100644 --- a/midscene-core/src/main/java/com/midscene/core/agent/Orchestrator.java +++ b/midscene-core/src/main/java/com/midscene/core/agent/Orchestrator.java @@ -123,7 +123,11 @@ public void execute(String instruction) { } } - history.add(UserMessage.from("Error executing plan: " + e.getMessage())); + String reflectionMsg = String.format( + "Previous execution failed with error: %s.\n" + + "Please reflect on what went wrong, verify the page state, and formulate a new plan " + + "to complete the instruction: %s", e.getMessage(), instruction); + history.add(UserMessage.from(reflectionMsg)); } } diff --git a/midscene-core/src/main/java/com/midscene/core/context/Context.java b/midscene-core/src/main/java/com/midscene/core/context/Context.java index b017c331..f0a95dcf 100644 --- a/midscene-core/src/main/java/com/midscene/core/context/Context.java +++ b/midscene-core/src/main/java/com/midscene/core/context/Context.java @@ -1,14 +1,13 @@ package com.midscene.core.context; -import java.util.ArrayList; -import java.util.Collections; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.List; import lombok.Getter; public class Context { @Getter - private final List events = Collections.synchronizedList(new ArrayList<>()); + private final List events = new CopyOnWriteArrayList<>(); public void logEvent(ContextEvent event) { events.add(event);