From e3d7646dcb5628e7fdc5bf0b57841039c00ad8ac Mon Sep 17 00:00:00 2001 From: anmolg1997 Date: Fri, 5 Jun 2026 00:51:39 +0530 Subject: [PATCH 1/2] feat(console): make `make console` self-sufficient, auto-open, clean teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make console` already ran both localhosts (backend :8742 + Vite dev server) and kept them alive — confirmed that's the "build both + keep running" command. Three fixes found while verifying it: - Auto-open the app once Vite is serving, reading the REAL url from Vite's own "Local:" banner (Vite bumps off a busy 5173, so a hardcoded port was wrong). - Self-sufficient: if v1/out has no discovery-*.json, render the offline golden suite first (no key, no cost) — a first `make console` no longer dies on the explorer's sync-data step ("cannot read v1/out") with a confusing Node error. - Robust teardown: start the backend + dev server each in their OWN process group and kill the group on Ctrl-C, so grandchildren (npm -> vite -> esbuild) are reaped too instead of leaking. Cross-platform (setsid/killpg on POSIX, CREATE_NEW_PROCESS_GROUP on Windows). README: give the Console its own paragraph (no key needed; opens the browser; Ctrl-C stops both). Verified end-to-end: both ports serve, opens the correct (bumped) port, SIGINT leaves 0 stray procs; empty-out auto-generates then starts. pyrefly clean on tasks.py; suite 248 pass. --- README.md | 7 +++- tasks.py | 96 ++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 3e0facf..4b38119 100644 --- a/README.md +++ b/README.md @@ -119,13 +119,18 @@ Want a **live** run (real agent, spends credits)? Add a key first: # put ANTHROPIC_API_KEY=… (or the AZURE_OPENAI_* vars) in v1/.env — setup seeds it from the template make doctor # (or: python tasks.py doctor) — verify the provider is reachable (one tiny call) make live # the real pipeline — minutes, costs credits -make console # OR: the interactive 6-stage Console (backend + UI dev server) ``` > A live run **preflights your credentials**: if `v1/.env` has no real key (or still has the > `.env.example` placeholder), it stops immediately with a one-line fix instead of a traceback or a > misleading "use the golden run" message. With no key, use `make run` for the full offline demo. +**Interactive Console** — the 6-stage operator flow with both servers running and the explorer +embedded (`make console` / `python tasks.py console`). It starts the backend (`:8742`) and the +explorer dev server together, **opens the app in your browser**, and keeps both running until you +Ctrl-C (which stops both). No key needed — if no report data exists yet it renders the offline +golden suite first; a live run is then a choice inside the Console UI. + Run on any domain by dropping its documents in `v1/inputs//`, then `make run DOMAIN=` (or `python tasks.py run --domain `). diff --git a/tasks.py b/tasks.py index aad34bb..fc2422d 100644 --- a/tasks.py +++ b/tasks.py @@ -21,7 +21,9 @@ import argparse import os +import re import shutil +import signal import subprocess import sys import webbrowser @@ -122,22 +124,94 @@ def live(a) -> None: _uv(["run", "python", "run.py", "--domain", a.domain, "--fresh", "--auto-resolve"]) -def console(_a) -> None: - """Start the backend (server.py) then the Vite dev server. Ctrl-C stops both.""" +# Vite prints its real URL ("➜ Local: http://localhost:5173/") and bumps to the next free port +# if 5173 is taken — so we read the URL from its own output rather than guessing the port. +_VITE_LOCAL_RE = re.compile(r"Local:\s*(https?://\S+?)/?\s*$") + + +def _has_report_data() -> bool: + """True if the engine has produced at least one synthesis JSON for the explorer to render. + The explorer's predev step (sync-data) hard-fails without one, so the Console needs this first.""" + out = V1 / "out" + return out.is_dir() and any(out.glob("discovery-*.json")) + + +def _popen_group(cmd: list[str], **kw) -> subprocess.Popen: + """Start a child in its OWN process group so we can later kill it AND its grandchildren + (npm -> vite -> esbuild) in one shot. Without this, terminating npm orphans esbuild.""" + if os.name == "nt": + kw["creationflags"] = kw.get("creationflags", 0) | subprocess.CREATE_NEW_PROCESS_GROUP + else: + kw["start_new_session"] = True # setsid: child becomes its own process-group leader + return subprocess.Popen(cmd, **kw) + + +def _kill_group(proc: subprocess.Popen) -> None: + """Terminate a child and every process in its group; fall back to a hard kill.""" + if proc.poll() is not None: + return + try: + if os.name == "nt": + proc.terminate() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + except (ProcessLookupError, PermissionError): + return + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + try: + if os.name == "nt": + proc.kill() + else: + os.killpg(os.getpgid(proc.pid), __import__("signal").SIGKILL) + except (ProcessLookupError, PermissionError): + pass + + +def console(a) -> None: + """Run both localhosts and keep them alive: the backend (server.py, :8742) and the explorer + dev server (Vite). Tees Vite's output, opens the exact URL Vite reports (handles a bumped port), + then leaves both running so you drive it. Ctrl-C stops both.""" _require("uv", "Install uv first: https://docs.astral.sh/uv/") _require("npm", "Install Node.js 20+ from https://nodejs.org") + # The explorer can't start without report data (its predev sync-data step exits 1 on an empty + # out/). If none exists yet, generate the offline golden suite first — no key, no cost — so a + # first `make console` just works instead of dying on a Node error. + if not _has_report_data(): + print("→ No report data yet — rendering the offline golden suite first (no key, no cost)…") + report(a) print("→ Starting Discovery Console backend on http://127.0.0.1:8742 …") - backend = subprocess.Popen(["uv", "run", "python", "server.py"], cwd=str(V1), env=_uv_env()) + backend = _popen_group(["uv", "run", "python", "server.py"], cwd=str(V1), env=_uv_env()) + + print("→ Starting the explorer dev server (Ctrl-C to stop both)…") + npm = shutil.which("npm") or "npm" + # Pipe stdout so we can read the real Local URL; tee every line straight back to the terminal so + # the dev server still looks/behaves normal (HMR logs, errors, the URL banner). Own process + # group so teardown reaps the grandchildren (vite/esbuild), not just npm. + vite = _popen_group([npm, "run", "dev"], cwd=str(UI), + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) + + opened = False try: - print("→ Starting the explorer dev server (Ctrl-C to stop both)…") - npm = shutil.which("npm") or "npm" - subprocess.run([npm, "run", "dev"], cwd=str(UI)) + assert vite.stdout is not None + for line in vite.stdout: + sys.stdout.write(line) + sys.stdout.flush() + if not opened: + m = _VITE_LOCAL_RE.search(line) + if m: + url = m.group(1) + print(f"→ Opening {url} (both servers stay running; Ctrl-C to stop)…") + webbrowser.open(url) + opened = True + vite.wait() + except KeyboardInterrupt: + pass finally: - backend.terminate() - try: - backend.wait(timeout=10) - except subprocess.TimeoutExpired: - backend.kill() + print("\n→ Stopping both servers…") + _kill_group(vite) + _kill_group(backend) def doctor(_a) -> None: From 352e6838de53dc76c2141da7c987565d07080278 Mon Sep 17 00:00:00 2001 From: anmolg1997 Date: Fri, 5 Jun 2026 01:18:07 +0530 Subject: [PATCH 2/2] fix(live): synthesis crash on string strategy_profile + Console backend robustness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated by a genuinely-fresh p2p E2E run (out/ renamed aside so nothing replays from prior output): the live agent ran, but synthesis aborted the whole suite with "'str' object is not a mapping". Root cause: a live section emits strategy_profile as a bare string; build.py's `{**strategy_profile}` spread chokes on a str. Golden/ cached runs never hit it because their emits are well-formed — so it only bites real live runs (exactly the path under test). - build.py: coerce strategy_profile to a dict at both the spread site and in _from_payload, so a stray string is discarded (the typed StrategyProfile still wins) instead of crashing. +2 regression tests (direct mapper + full fan-out). Suite 250 pass, 100% branch coverage held. Also fixes the UI "⚠ Failed to fetch": its root cause is a stale Console backend still holding :8742, which made a new server.py crash silently (OSError: address in use) — Vite then started, so the UI loaded with no backend to call. - server.py: catch EADDRINUSE and exit 1 with a clear "port in use — stop the other backend or set DISCOVERY_UI_PORT" message (no traceback); allow_reuse_address so a just-stopped backend's TIME_WAIT socket doesn't block restart. - tasks.py (console): poll /healthz until the backend answers BEFORE starting Vite; if it never comes up, abort with the stop-the-stale-one hint instead of opening a UI that can only say "Failed to fetch". Verified end-to-end: fresh p2p run.py --fresh → full 6-report suite (4 findings); and the Console path (POST /api/run mode=live → SSE) streams real live agent activity. pyrefly clean on product code + tasks.py. --- tasks.py | 35 ++++++++++++++++++++++++++++++- v1/discovery/reportsuite/build.py | 12 ++++++++--- v1/server.py | 23 ++++++++++++++++++-- v1/tests/test_build.py | 35 +++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 6 deletions(-) diff --git a/tasks.py b/tasks.py index fc2422d..7b4a8d8 100644 --- a/tasks.py +++ b/tasks.py @@ -26,6 +26,8 @@ import signal import subprocess import sys +import time +import urllib.request import webbrowser from pathlib import Path @@ -128,6 +130,26 @@ def live(a) -> None: # if 5173 is taken — so we read the URL from its own output rather than guessing the port. _VITE_LOCAL_RE = re.compile(r"Local:\s*(https?://\S+?)/?\s*$") +# The Console backend (server.py). Keep in sync with DISCOVERY_UI_PORT / server.py's default. +_BACKEND_PORT = int(os.environ.get("DISCOVERY_UI_PORT", "8742")) +_BACKEND_HEALTH = f"http://127.0.0.1:{_BACKEND_PORT}/healthz" + + +def _wait_backend(proc: subprocess.Popen, timeout: float = 20.0) -> bool: + """Poll the backend's /healthz until it answers (True) or it dies / times out (False). + Starting Vite only after the backend is reachable is what prevents the UI from loading + against a dead backend and showing 'Failed to fetch' the moment you click Run.""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if proc.poll() is not None: + return False # backend exited (e.g. port in use) — caller surfaces why + try: + with urllib.request.urlopen(_BACKEND_HEALTH, timeout=1): + return True + except Exception: + time.sleep(0.4) + return False + def _has_report_data() -> bool: """True if the engine has produced at least one synthesis JSON for the explorer to render. @@ -181,9 +203,20 @@ def console(a) -> None: if not _has_report_data(): print("→ No report data yet — rendering the offline golden suite first (no key, no cost)…") report(a) - print("→ Starting Discovery Console backend on http://127.0.0.1:8742 …") + print(f"→ Starting Discovery Console backend on http://127.0.0.1:{_BACKEND_PORT} …") backend = _popen_group(["uv", "run", "python", "server.py"], cwd=str(V1), env=_uv_env()) + # Wait until the backend is actually answering before starting the UI. If it never comes up + # (the common cause is a stale backend already holding the port — server.py prints that and + # exits 1), abort with a clear message instead of opening a UI that can only say "Failed to + # fetch". server.py's own stderr (incl. the port-in-use hint) is inherited, so it's visible. + if not _wait_backend(backend): + _kill_group(backend) + sys.exit(f"✗ The Console backend did not come up on port {_BACKEND_PORT} (see its message " + f"above).\n If a previous Console is still running, stop it: " + f"lsof -ti tcp:{_BACKEND_PORT} | xargs kill (then re-run).") + print("✓ Backend is up.") + print("→ Starting the explorer dev server (Ctrl-C to stop both)…") npm = shutil.which("npm") or "npm" # Pipe stdout so we can read the real Local URL; tee every line straight back to the terminal so diff --git a/v1/discovery/reportsuite/build.py b/v1/discovery/reportsuite/build.py index 0955d40..1cea206 100644 --- a/v1/discovery/reportsuite/build.py +++ b/v1/discovery/reportsuite/build.py @@ -50,8 +50,12 @@ def build_synthesis(raw_payload: dict, *, domain: str = "o2c", live=False, llm=N content.fact_store = fs content.strategy = strat content.planning_assumptions = planning - # surface only the NON-EMPTY strategy fields alongside r05's posture (don't blank anything) - content.strategy_profile = {**content.strategy_profile, + # surface only the NON-EMPTY strategy fields alongside r05's posture (don't blank anything). + # Defensive: a live section occasionally emits strategy_profile as a bare string instead of + # an object — spreading that would raise "'str' object is not a mapping" and abort the whole + # suite. Coerce to a dict (a stray string is discarded; the typed StrategyProfile wins). + base = content.strategy_profile if isinstance(content.strategy_profile, dict) else {} + content.strategy_profile = {**base, **{k: v for k, v in strat.to_dict().items() if v}} elif live: from .. import synthesis @@ -1243,7 +1247,9 @@ def _from_payload(payload: dict) -> SynthesisContent: opportunities=opps, sequencing_rationale=tr.get("sequencing_rationale", ""), strategic_readiness=tr.get("strategic_readiness", ""), dependency_notes=tr.get("dependency_notes", ""), roadmap=roadmap, - strategy_profile=payload.get("strategy_profile", {}), + # guard: the model may emit strategy_profile as a string — keep it a dict so downstream + # spreads/renders never choke (the live builder overlays the typed StrategyProfile anyway). + strategy_profile=sp if isinstance((sp := payload.get("strategy_profile", {})), dict) else {}, metrics_framework=[MetricItem(name=m["name"], definition=m.get("definition", ""), target=m.get("target", "")) for m in payload.get("metrics_framework", [])], diff --git a/v1/server.py b/v1/server.py index 8885eaa..062c800 100644 --- a/v1/server.py +++ b/v1/server.py @@ -268,9 +268,28 @@ def _static(self, rel: str) -> None: self._send(200, target.read_bytes(), ctype) +class _Server(ThreadingHTTPServer): + # Avoid a TIME_WAIT socket from a just-stopped backend blocking an immediate restart. + allow_reuse_address = True + + def main() -> int: - srv = ThreadingHTTPServer(("127.0.0.1", PORT), Handler) - print(f"Discovery Console backend on http://127.0.0.1:{PORT} (Ctrl-C to stop)") + try: + srv = _Server(("127.0.0.1", PORT), Handler) + except OSError as e: + # The usual cause: a previous Console backend is still running on this port. Fail with a + # clear, actionable message instead of a raw traceback (which the UI would see as a silent + # "Failed to fetch" — the backend never came up). + import errno + if e.errno == errno.EADDRINUSE: + print(f"error: port {PORT} is already in use — another Discovery Console backend is " + f"probably running.\n" + f" Stop it (find it with: lsof -ti tcp:{PORT} | xargs kill), or start this one " + f"on a different port: DISCOVERY_UI_PORT=8743 uv run python server.py", + flush=True) + return 1 + raise + print(f"Discovery Console backend on http://127.0.0.1:{PORT} (Ctrl-C to stop)", flush=True) try: srv.serve_forever() except KeyboardInterrupt: diff --git a/v1/tests/test_build.py b/v1/tests/test_build.py index 8f4a704..f9ea4d8 100644 --- a/v1/tests/test_build.py +++ b/v1/tests/test_build.py @@ -141,6 +141,41 @@ def test_from_payload_tolerates_missing_optional_keys(): assert content.opportunities == [] +def test_strategy_profile_as_string_does_not_crash(): + """Regression: a live run once aborted the WHOLE suite with 'str object is not a mapping' when + a section emitted strategy_profile as a bare string instead of an object — the {**profile} + spread choked on it. _from_payload must coerce a non-dict strategy_profile to {} so neither the + mapper nor the downstream spread crashes.""" + payload = { + "current_state": {"domain_overview": "x"}, "pain_points": [], "opportunities": [], + "transformation": {}, "roadmap": [], + "strategy_profile": "Acme should prioritise PO compliance.", # <- a STRING, not an object + } + content = build._from_payload(payload) + assert content.strategy_profile == {} # coerced, not the offending string + # and the dict form still passes through untouched + ok = build._from_payload({**payload, "strategy_profile": {"direction_type": "consolidate"}}) + assert ok.strategy_profile == {"direction_type": "consolidate"} + + +class _FanoutLLMStringProfile(_FanoutLLM): + """A fan-out fake whose recommendation section emits strategy_profile as a bare STRING — the + exact malformed shape that aborted a live p2p suite before the coercion fix.""" + _EMITS = {**_FanoutLLM._EMITS, + "emit_recommendation": {**_FanoutLLM._EMITS["emit_recommendation"], + "strategy_profile": "Acme should prioritise PO compliance."}} + + +def test_fanout_survives_string_strategy_profile(): + """End-to-end through the deep fan-out: even if a section emits strategy_profile as a string, + build_synthesis completes (the live builder overlays the typed StrategyProfile).""" + reg = {"csv_ids": [], "doc_ids": [], "manifest": {"strategy_profile": { + "direction_type": "consolidate", "horizon": "0-6 months"}}} + content = build.build_synthesis(_raw(), domain="o2c", live=True, + llm=_FanoutLLMStringProfile(), doc_keys=[], reg=reg) + assert content.strategy_profile.get("direction_type") == "consolidate" # typed profile wins + + def test_no_fixture_for_unknown_domain_refuses(): with pytest.raises(build.NoFixtureForDomain, match="no grounded fixture"): build.build_synthesis(_raw(), domain="p2p", live=False)