Fix submission review issues

CHANGELOG_AND_RUNBOOK.md

@@ -5,7 +5,7 @@ This file explains what changed in the project and how to run or test each part.
 Project path:
 
 ```bash
-cd /Users/adityagaba/Downloads/incident-triage-env
+cd <repo-root>
 ```
 
 ## 1. What changed
@@ -126,7 +126,7 @@ These logs appear in the same terminal where `uvicorn` is running.
 Use port `8000` locally if port `7860` is busy.
 
 ```bash
-cd /Users/adityagaba/Downloads/incident-triage-env
+cd <repo-root>
 source .venv/bin/activate
 .venv/bin/python -m uvicorn app:app --host 127.0.0.1 --port 8000
 ```
@@ -372,7 +372,7 @@ Expected:
 ## 6. Run automated tests
 
 ```bash
-cd /Users/adityagaba/Downloads/incident-triage-env
+cd <repo-root>
 .venv/bin/python -m unittest discover -s tests -v
 ```
 
@@ -385,7 +385,7 @@ OK
 ## 7. Run OpenEnv local validation
 
 ```bash
-cd /Users/adityagaba/Downloads/incident-triage-env
+cd <repo-root>
 .venv/bin/openenv validate . --json
 ```
 
@@ -400,7 +400,7 @@ Expected:
 If the local app is running on port `8000`:
 
 ```bash
-cd /Users/adityagaba/Downloads/incident-triage-env
+cd <repo-root>
 ENV_URL=http://127.0.0.1:8000 .venv/bin/python inference.py
 ```
 

README.md

@@ -95,7 +95,7 @@ The project also serves a browser-facing UI from the same FastAPI app:
 
 ## Models
 
-The core models are defined in [models.py](/Users/adityagaba/Downloads/incident-triage-env/models.py):
+The core models are defined in [models.py](./models.py):
 
 - `IncidentObservation`
 - `IncidentAction`
@@ -112,7 +112,7 @@ Validation rules:
 
 ## Reward Logic
 
-Rewarding is deterministic and implemented in [graders.py](/Users/adityagaba/Downloads/incident-triage-env/graders.py).
+Rewarding is deterministic and implemented in [graders.py](./graders.py).
 
 - `task1`: `1.0` exact, `0.5` adjacent severity, `0.0` far miss
 - `task2`: `1.0` exact, `0.5` related domain, `0.25` `UNKNOWN`, `0.0` wrong

app.py

@@ -2,6 +2,7 @@ import uuid
 from collections import Counter
 from pathlib import Path
 import sys
+from threading import RLock
 from typing import Any
 
 from fastapi import FastAPI, HTTPException
@@ -25,7 +26,10 @@ UI_DIR = Path(__file__).parent / "ui"
 ASSETS_DIR = UI_DIR / "assets"
 
 # Session store: session_id -> IncidentEnv instance
+MAX_SESSIONS = 500
 sessions: dict[str, IncidentEnv] = {}
+completed_states: dict[str, IncidentState] = {}
+session_lock = RLock()
 task_counts = Counter(ticket["task_type"] for ticket in TICKETS)
 
 app.mount("/assets", StaticFiles(directory=ASSETS_DIR), name="assets")
@@ -36,6 +40,14 @@ def log_event(event: str, **fields: Any) -> None:
     print(f"[{event}] {details}", file=sys.stderr, flush=True)
 
 
+def evict_oldest(mapping: dict[str, Any], max_size: int) -> None:
+    while len(mapping) >= max_size:
+        oldest_key = next(iter(mapping), None)
+        if oldest_key is None:
+            return
+        mapping.pop(oldest_key, None)
+
+
 @app.get("/", include_in_schema=False)
 def home_page():
     return FileResponse(UI_DIR / "index.html")
@@ -140,7 +152,10 @@ def reset(reset_request: ResetRequest | None = None):
             error=str(e),
         )
         raise HTTPException(status_code=400, detail=str(e))
-    sessions[session_id] = env
+    with session_lock:
+        evict_oldest(sessions, MAX_SESSIONS)
+        evict_oldest(completed_states, MAX_SESSIONS)
+        sessions[session_id] = env
     result.info["session_id"] = session_id
     result.info["state"] = env.state(session_id=session_id).model_dump()
     log_event(
@@ -155,17 +170,25 @@ def reset(reset_request: ResetRequest | None = None):
 
 @app.post("/step", response_model=StepResult)
 def step(action: IncidentAction, session_id: str):
-    env = sessions.get(session_id)
-    if not env:
-        log_event("STEP_ERROR", session_id=session_id, error="session_not_found")
-        raise HTTPException(status_code=404, detail="Session not found. Call /reset first.")
-    try:
-        result = env.step(action)
-    except (RuntimeError, ValueError) as e:
-        log_event("STEP_ERROR", session_id=session_id, incident_id=action.incident_id, error=str(e))
-        raise HTTPException(status_code=400, detail=str(e))
-    result.info["session_id"] = session_id
-    result.info["state"] = env.state(session_id=session_id).model_dump()
+    with session_lock:
+        env = sessions.get(session_id)
+        if not env:
+            if session_id in completed_states:
+                log_event("STEP_ERROR", session_id=session_id, error="episode_already_completed")
+                raise HTTPException(status_code=400, detail="Episode already completed. Call reset() to start a new one.")
+            log_event("STEP_ERROR", session_id=session_id, error="session_not_found")
+            raise HTTPException(status_code=404, detail="Session not found. Call /reset first.")
+        try:
+            result = env.step(action)
+        except (RuntimeError, ValueError) as e:
+            log_event("STEP_ERROR", session_id=session_id, incident_id=action.incident_id, error=str(e))
+            raise HTTPException(status_code=400, detail=str(e))
+        result.info["session_id"] = session_id
+        current_state = env.state(session_id=session_id)
+        result.info["state"] = current_state.model_dump()
+        if result.done:
+            completed_states[session_id] = current_state
+            sessions.pop(session_id, None)
     log_event(
         "STEP",
         session_id=session_id,
@@ -180,17 +203,22 @@ def step(action: IncidentAction, session_id: str):
 
 @app.get("/state", response_model=IncidentState)
 def state(session_id: str):
-    env = sessions.get(session_id)
-    if not env:
-        log_event("STATE_ERROR", session_id=session_id, error="no_active_session")
-        raise HTTPException(status_code=404, detail="No active session.")
-    try:
-        current_state = env.state(session_id=session_id)
-        log_event("STATE", session_id=session_id, incident_id=current_state.incident_id, done=str(current_state.done).lower())
-        return current_state
-    except RuntimeError as e:
-        log_event("STATE_ERROR", session_id=session_id, error=str(e))
-        raise HTTPException(status_code=404, detail=str(e))
+    with session_lock:
+        env = sessions.get(session_id)
+        if not env:
+            completed_state = completed_states.get(session_id)
+            if completed_state:
+                log_event("STATE", session_id=session_id, incident_id=completed_state.incident_id, done=str(completed_state.done).lower())
+                return completed_state
+            log_event("STATE_ERROR", session_id=session_id, error="no_active_session")
+            raise HTTPException(status_code=404, detail="No active session.")
+        try:
+            current_state = env.state(session_id=session_id)
+            log_event("STATE", session_id=session_id, incident_id=current_state.incident_id, done=str(current_state.done).lower())
+            return current_state
+        except RuntimeError as e:
+            log_event("STATE_ERROR", session_id=session_id, error=str(e))
+            raise HTTPException(status_code=404, detail=str(e))
 
 
 @app.get("/grader")

inference.py

@@ -325,6 +325,7 @@ def run_episode(
     steps_taken = 0
     score = 0.0
     success = False
+    episode_result: Dict[str, Any]
 
     log_start(task=ticket["incident_id"], env=BENCHMARK, model=active_model_name(model_client))
 
@@ -349,9 +350,8 @@ def run_episode(
             done=bool(step_data.get("done", True)),
             error=None,
         )
-        log_end(success=success, steps=steps_taken, score=score, rewards=rewards)
 
-        return {
+        episode_result = {
             "incident_id": ticket["incident_id"],
             "task_type": ticket["task_type"],
             "difficulty": observation.get("difficulty"),
@@ -362,14 +362,19 @@ def run_episode(
         }
     except Exception as exc:
         log_step(step=max(steps_taken, 1), action="error", reward=0.0, done=True, error=str(exc))
-        log_end(success=False, steps=steps_taken, score=0.0, rewards=rewards)
-        return {
+        score = 0.0
+        success = False
+        episode_result = {
             "incident_id": ticket["incident_id"],
             "task_type": ticket["task_type"],
             "score": 0.0,
             "success": False,
             "error": str(exc),
         }
+    finally:
+        log_end(success=success, steps=max(steps_taken, 1), score=score, rewards=rewards or [0.0])
+
+    return episode_result
 
 
 def write_results(results: List[Dict[str, Any]]) -> None:

pyproject.toml

@@ -37,3 +37,7 @@ py-modules = [
     "inference",
     "models",
 ]
+
+[tool.setuptools.packages.find]
+where = ["."]
+include = ["server*"]

tests/test_env.py

@@ -2,16 +2,18 @@ import unittest
 
 from fastapi.testclient import TestClient
 
-from app import app, sessions
+from app import app, completed_states, sessions
 
 
 class IncidentEnvApiTests(unittest.TestCase):
     def setUp(self) -> None:
         sessions.clear()
+        completed_states.clear()
         self.client = TestClient(app)
 
     def tearDown(self) -> None:
         sessions.clear()
+        completed_states.clear()
 
     def test_health_schema_and_mcp_helper_endpoints(self) -> None:
         health_response = self.client.get("/health")
@@ -101,6 +103,19 @@ class IncidentEnvApiTests(unittest.TestCase):
         self.assertTrue(state_body["done"])
         self.assertEqual(state_body["status"], "completed")
         self.assertEqual(state_body["last_reward"], 1.0)
+        self.assertNotIn(session_id, sessions)
+        self.assertIn(session_id, completed_states)
+
+        repeated_step_response = self.client.post(
+            f"/step?session_id={session_id}",
+            json={
+                "incident_id": "INC-014",
+                "task_type": "task3",
+                "action": "FAILOVER",
+            },
+        )
+        self.assertEqual(repeated_step_response.status_code, 400)
+        self.assertIn("already completed", repeated_step_response.json()["detail"])
 
     def test_step_rejects_action_for_wrong_task_type(self) -> None:
         reset_response = self.client.post(

ui/assets/app.js

@@ -13,6 +13,13 @@ function safeText(value) {
   return value == null ? "--" : String(value);
 }
 
+function createBadge(text, extraClass = "") {
+  const badge = document.createElement("span");
+  badge.className = extraClass ? `badge ${extraClass}` : "badge";
+  badge.textContent = safeText(text);
+  return badge;
+}
+
 function setHealthPill(status) {
   const pills = document.querySelectorAll("[data-health-pill]");
   pills.forEach((pill) => {
@@ -23,22 +30,36 @@ function setHealthPill(status) {
 
 function renderTaskCards(target, tasks) {
   if (!target) return;
-  target.innerHTML = "";
+  target.replaceChildren();
   Object.entries(tasks).forEach(([taskId, task]) => {
     const article = document.createElement("article");
     article.className = "task-card";
-    article.innerHTML = `
-      <span class="badge difficulty-${task.difficulty}">${task.difficulty}</span>
-      <h3>${task.name}</h3>
-      <p>Expected field: <strong>${task.expected_field || task.output_field}</strong></p>
-      <div class="task-meta">
-        <span class="badge">${taskId}</span>
-        <span class="badge">${task.ticket_count || 0} incidents</span>
-      </div>
-      <div class="task-values">
-        ${(task.allowed_values || task.labels || []).map((value) => `<span class="badge">${value}</span>`).join("")}
-      </div>
-    `;
+
+    const difficultyClass = `difficulty-${safeText(task.difficulty).toLowerCase().replace(/[^a-z0-9_-]/g, "")}`;
+    const difficulty = createBadge(task.difficulty, difficultyClass);
+    const title = document.createElement("h3");
+    title.textContent = safeText(task.name);
+
+    const expectedField = document.createElement("p");
+    expectedField.append("Expected field: ");
+    const expectedFieldValue = document.createElement("strong");
+    expectedFieldValue.textContent = safeText(task.expected_field || task.output_field);
+    expectedField.appendChild(expectedFieldValue);
+
+    const taskMeta = document.createElement("div");
+    taskMeta.className = "task-meta";
+    taskMeta.append(
+      createBadge(taskId),
+      createBadge(`${task.ticket_count || 0} incidents`),
+    );
+
+    const taskValues = document.createElement("div");
+    taskValues.className = "task-values";
+    (task.allowed_values || task.labels || []).forEach((value) => {
+      taskValues.appendChild(createBadge(value));
+    });
+
+    article.append(difficulty, title, expectedField, taskMeta, taskValues);
     target.appendChild(article);
   });
 }
@@ -69,15 +90,21 @@ async function initStatus() {
   renderTaskCards(document.querySelector("[data-task-grid]"), metadata.tasks);
 
   const schemaGrid = document.querySelector("[data-schema-grid]");
-  schemaGrid.innerHTML = Object.keys(schema)
-    .map((name) => `<span class="badge">${name}</span>`)
-    .join("");
+  schemaGrid.replaceChildren();
+  Object.keys(schema).forEach((name) => {
+    schemaGrid.appendChild(createBadge(name));
+  });
 
   document.querySelector("[data-grader-summary]").textContent = grader.scoring;
   const graderList = document.querySelector("[data-grader-list]");
-  graderList.innerHTML = Object.entries(grader.tasks)
-    .map(([task, rule]) => `<li><strong>${task}</strong>: ${rule}</li>`)
-    .join("");
+  graderList.replaceChildren();
+  Object.entries(grader.tasks).forEach(([task, rule]) => {
+    const item = document.createElement("li");
+    const taskName = document.createElement("strong");
+    taskName.textContent = task;
+    item.append(taskName, `: ${safeText(rule)}`);
+    graderList.appendChild(item);
+  });
 }
 
 function buildActionPayload(observation, selectedValue) {
@@ -160,9 +187,13 @@ async function initPlayground() {
   try {
     const ticketData = await fetchJson("/tickets");
     validTickets = ticketData.tickets || [];
-    ticketOptions.innerHTML = validTickets
-      .map((ticket) => `<option value="${ticket.incident_id}" label="${ticket.task_type} / ${ticket.task_name}"></option>`)
-      .join("");
+    ticketOptions.replaceChildren();
+    validTickets.forEach((ticket) => {
+      const option = document.createElement("option");
+      option.value = safeText(ticket.incident_id);
+      option.label = `${safeText(ticket.task_type)} / ${safeText(ticket.task_name)}`;
+      ticketOptions.appendChild(option);
+    });
     ticketHelper.textContent = `Valid ticket range: ${validTickets[0]?.incident_id || "--"} to ${validTickets.at(-1)?.incident_id || "--"}.`;
   } catch (error) {
     ticketHelper.textContent = `Could not load ticket list: ${error.message}`;
@@ -217,9 +248,13 @@ async function initPlayground() {
       expectedFieldInput.value = observation.expected_field;
       actionValueSelect.disabled = false;
       stepButton.disabled = false;
-      actionValueSelect.innerHTML = observation.allowed_values
-        .map((value) => `<option value="${value}">${value}</option>`)
-        .join("");
+      actionValueSelect.replaceChildren();
+      observation.allowed_values.forEach((value) => {
+        const option = document.createElement("option");
+        option.value = safeText(value);
+        option.textContent = safeText(value);
+        actionValueSelect.appendChild(option);
+      });
 
       setOutput(observationOutput, result);
       setOutput(resultOutput, "No step submitted yet.");
@@ -282,7 +317,12 @@ async function bootstrap() {
     const pageShell = document.querySelector(".page-shell");
     const banner = document.createElement("div");
     banner.className = "floating-panel";
-    banner.innerHTML = `<strong>UI data load failed.</strong><p class="status-helper">${error.message}</p>`;
+    const title = document.createElement("strong");
+    title.textContent = "UI data load failed.";
+    const detail = document.createElement("p");
+    detail.className = "status-helper";
+    detail.textContent = error.message;
+    banner.append(title, detail);
     pageShell?.prepend(banner);
   }
 }