Add architectural and JS-parity review document

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).

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,38 +396,37 @@ 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);
         if (result instanceof Boolean) {
           shouldCheck = (Boolean) result;
         }
       } catch (Exception e) {
-        // Fallback to true if script fails
         shouldCheck = true;
       }
 
       if (shouldCheck) {
         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<Void> aiActionAsync(String instruction) {
-    return CompletableFuture.runAsync(() -> aiAction(instruction));
+    CompletableFuture<Void> 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 ==========

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));
       }
     }
 

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<ContextEvent> events = Collections.synchronizedList(new ArrayList<>());
+  private final List<ContextEvent> events = new CopyOnWriteArrayList<>();
 
   public void logEvent(ContextEvent event) {
     events.add(event);