.

docs/rolling_feedback.md

@@ -0,0 +1,187 @@
+This is a well-structured application. You have successfully separated concerns (Actions vs. API vs. Browser Control vs. Domain logic), effectively used **Pydantic** for data validation, and established a clear pattern for your automation logic (the **Action pattern**).
+
+However, as you scale from a "Demo" to a larger production system with dozens of actions and higher concurrency, specific architectural choices in this scaffold will become bottlenecks.
+
+Here is a critique of your project structure with actionable design decisions for scalability and organization.
+
+---
+
+### 1. The "Strings" Abstraction: Runtime vs. Static Analysis
+**Location:** `src/guide/app/strings/`
+
+**Critique:**
+You have a sophisticated system (`service.py`) that dynamically resolves strings using `getattr`:
+```python
+# Current approach
+description_val = strings.text("INTAKE", "CONVEYOR_BELT_REQUEST")
+```
+While this centralizes text, it breaks **Static Analysis** and **IDE Autocompletion**.
+1.  If you typo `"INTAKE"`, you won't know until runtime.
+2.  Refactoring tools (Rename Symbol) won't work across your codebase.
+3.  The `_SELECTORS`, `_LABELS` dict mappings need manual updating.
+
+**Design Decision:**
+Replace the dynamic lookup service with **Static Nested Classes** or **Pydantic Models**. This keeps the centralization but restores developer ergonomics.
+
+**Proposed Change:**
+```python
+# src/guide/app/strings/registry.py
+class IntakeStrings:
+    description_field = '[data-test="intake-description"]'
+    conveyor_request = "Requesting a conveyor belt..."
+
+class AppStrings:
+    intake = IntakeStrings()
+    auth = AuthStrings()
+
+strings = AppStrings()
+
+# Usage (Type-safe, Autocompletable)
+val = strings.intake.conveyor_request
+```
+
+### 2. Action Registry Scalability (The Open/Closed Principle)
+**Location:** `src/guide/app/actions/registry.py`
+
+**Critique:**
+Currently, you manually import and instantiate every action in `registry.py`:
+```python
+actions: list[DemoAction] = [
+    LoginAsPersonaAction(...),
+    FillIntakeBasicAction(),
+    # ... as this grows to 100 actions, this file becomes a merge-conflict nightmare
+]
+```
+This violates the Open/Closed Principle. To add a new action, you must modify the registry file.
+
+**Design Decision:**
+Use a **Decorator-based Registration** pattern or **Dynamic Discovery**.
+
+**Proposed Change:**
+Create a decorator that registers the class to a singleton registry upon import.
+
+```python
+# In src/guide/app/actions/base.py
+action_registry = {}
+
+def register_action(cls):
+    action_registry[cls.id] = cls
+    return cls
+
+# In src/guide/app/actions/intake/basic.py
+@register_action
+class FillIntakeBasicAction(DemoAction):
+    id = "fill-intake-basic"
+    # ...
+```
+*Then, in `main.py`, you simply import the `actions` package, and the registry populates automatically.*
+
+### 3. Browser Resource Management (Performance)
+**Location:** `src/guide/app/browser/client.py`
+
+**Critique:**
+Your `open_page` context manager appears to launch a browser or connect via CDP for *every single action request*:
+```python
+browser = await playwright.chromium.connect_over_cdp(cdp_url)
+# ...
+await browser.close()
+```
+Browser startup/connection is the most expensive part of automation. If you receive 10 requests/second, this will choke the host machine.
+
+**Design Decision:**
+Implement **Browser Context Pooling**.
+1.  The Application startup should initialize the `Browser` object (keep the connection open).
+2.  Each `Action` request should only create a new `BrowserContext` (incognito window equivalent) or `Page`.
+3.  Closing a Context is instant; closing a Browser is slow.
+
+### 4. Configuration Complexity
+**Location:** `src/guide/app/core/config.py`
+
+**Critique:**
+You have written significant custom logic (`_normalize_records`, `_coerce_mapping`) to handle YAML/JSON loading and merging. This is "infrastructure code" that you have to maintain and debug.
+
+**Design Decision:**
+Offload this to libraries designed for it. Since you are already using Pydantic, use **`pydantic-settings`** with standard loaders, or **Hydra** if the config is hierarchical.
+
+Minimize custom parsing logic. If you need to support dynamic personas/hosts, consider loading them from the database or a simple JSON file without trying to merge/normalize complex structures manually.
+
+### 5. GraphQL Type Safety
+**Location:** `src/guide/app/raindrop/`
+
+**Critique:**
+Your GraphQL client returns untyped dictionaries:
+```python
+# src/guide/app/raindrop/operations/sourcing.py
+result = data.get("createIntakeRequest") # result is JSONValue (Any)
+```
+As the external API changes, your code will break silently.
+
+**Design Decision:**
+Use **Code Generation**. Tools like `ariadne-codegen` or `gql` can read the Raindrop GraphQL schema (`schema.graphql`) and your query strings, then generate Pydantic models for the responses.
+
+**Result:**
+```python
+# Instead of dict access
+result = await client.create_intake(...)
+print(result.create_intake_request.id) # Fully typed
+```
+
+### 6. Dependency Injection (DI) Strategy
+**Location:** `src/guide/app/api/routes/actions.py`
+
+**Critique:**
+You are using a mix of approaches.
+1.  `app.state` accessed via `Request`.
+2.  Dependencies passed into Action `__init__`.
+3.  Context passed into Action `run`.
+
+The usage of `cast(AppStateProtocol, cast(object, app.state))` is verbose and un-Pythonic.
+
+**Design Decision:**
+Standardize on **FastAPI Dependency Injection**.
+Create a strictly typed `Deps` module.
+
+```python
+# src/guide/app/deps.py
+def get_registry(request: Request) -> ActionRegistry:
+    return request.app.state.registry
+
+# In router
+@router.post("/actions")
+async def run_action(
+    registry: Annotated[ActionRegistry, Depends(get_registry)],
+    # ...
+):
+    pass
+```
+Remove `Action` instantiation from the registry. The Registry should hold *Classes*, and the Router should instantiate them, injecting dependencies (like the Browser Client) at runtime. This makes unit testing Actions significantly easier because you don't have to mock the entire registry setup.
+
+### Summary of Recommended Structure Changes
+
+```text
+src/guide/app/
+  actions/
+    # Use decorators for registration
+    registry.py      <-- Logic to hold class references, not instances
+    base.py          <-- Base class handles common DI logic
+  browser/
+    pool.py          <-- NEW: Manages long-lived Browser connections
+    client.py        <-- Requests a Page from the Pool
+  strings/
+    # Refactor to static classes/Pydantic models
+    definitions.py   <-- Actual string data
+  raindrop/
+    codegen/         <-- Generated Pydantic models for GraphQL
+```
+
+### Final Verdict
+The project is currently at **Level 2 (Robust Prototype)**.
+*   It works.
+*   It's readable.
+*   It handles errors well.
+
+To get to **Level 3 (Production Scale)**, you must:
+1.  Remove dynamic string lookups (for DX).
+2.  Pool browser connections (for Performance).
+3.  Automate action registration (for Maintenance).
+4.  Generate GraphQL types (for Reliability).