Tarkeshwar Claude Opus 4.6 (1M context) commited on
Commit
febcf68
·
1 Parent(s): c2ad419

Fix critical issues from code review

Browse files

- Dockerfile: add non-root user (required by HF Spaces)
- app.py: add asyncio lock for concurrent request safety
- environment.py: fix score inconsistency — use _recompute_score()
consistently instead of ad-hoc per-step score manipulation
- environment.py: fix validate_row_deleted false positive — use index
tracking instead of content matching for duplicate detection
- client.py: fix step payload to wrap action in {"action": {...}}
- Remove duplicate data_clean_env/inference.py (only root one needed)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Dockerfile CHANGED
@@ -7,9 +7,6 @@ RUN apt-get update && \
7
  apt-get install -y --no-install-recommends git curl && \
8
  rm -rf /var/lib/apt/lists/*
9
 
10
- # Copy everything
11
- COPY . /app
12
-
13
  # Install Python dependencies
14
  RUN pip install --no-cache-dir \
15
  "openenv-core>=0.2.3" \
@@ -19,7 +16,14 @@ RUN pip install --no-cache-dir \
19
  "requests>=2.31.0" \
20
  "openai>=1.0.0"
21
 
22
- ENV PYTHONPATH="/app:$PYTHONPATH"
 
 
 
 
 
 
 
23
 
24
  HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \
25
  CMD python -c "import urllib.request; urllib.request.urlopen('http://localhost:8000/health')" || exit 1
 
7
  apt-get install -y --no-install-recommends git curl && \
8
  rm -rf /var/lib/apt/lists/*
9
 
 
 
 
10
  # Install Python dependencies
11
  RUN pip install --no-cache-dir \
12
  "openenv-core>=0.2.3" \
 
16
  "requests>=2.31.0" \
17
  "openai>=1.0.0"
18
 
19
+ # Copy everything
20
+ COPY . /app
21
+
22
+ # Create non-root user (required by HF Spaces)
23
+ RUN useradd -m -u 1000 user && chown -R user:user /app
24
+ USER user
25
+
26
+ ENV PYTHONPATH="/app"
27
 
28
  HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \
29
  CMD python -c "import urllib.request; urllib.request.urlopen('http://localhost:8000/health')" || exit 1
data_clean_env/client.py CHANGED
@@ -20,7 +20,7 @@ class DataCleanEnv(EnvClient[DataCleanAction, DataCleanObservation, DataCleanSta
20
  """
21
 
22
  def _step_payload(self, action: DataCleanAction) -> dict:
23
- return action.model_dump()
24
 
25
  def _parse_result(self, payload: dict) -> StepResult[DataCleanObservation]:
26
  obs = DataCleanObservation(**payload.get("observation", payload))
 
20
  """
21
 
22
  def _step_payload(self, action: DataCleanAction) -> dict:
23
+ return {"action": action.model_dump()}
24
 
25
  def _parse_result(self, payload: dict) -> StepResult[DataCleanObservation]:
26
  obs = DataCleanObservation(**payload.get("observation", payload))
data_clean_env/inference.py DELETED
@@ -1,284 +0,0 @@
1
- """
2
- Inference Script — DataCleanEnv
3
- ===================================
4
- MANDATORY
5
- - Before submitting, ensure the following variables are defined in your environment configuration:
6
- API_BASE_URL The API endpoint for the LLM.
7
- MODEL_NAME The model identifier to use for inference.
8
- HF_TOKEN Your Hugging Face / API key.
9
-
10
- - The inference script must be named `inference.py` and placed in the root directory of the project
11
- - Participants must use OpenAI Client for all LLM calls using above variables
12
- """
13
-
14
- import json
15
- import os
16
- import re
17
- import sys
18
- import textwrap
19
- from typing import Any, Dict, List
20
-
21
- from openai import OpenAI
22
-
23
- # ---------------------------------------------------------------------------
24
- # Config from environment
25
- # ---------------------------------------------------------------------------
26
- API_BASE_URL = os.getenv("API_BASE_URL", "https://router.huggingface.co/v1")
27
- API_KEY = os.getenv("HF_TOKEN") or os.getenv("API_KEY", "")
28
- MODEL_NAME = os.getenv("MODEL_NAME", "")
29
- ENV_URL = os.getenv("ENV_URL", "http://localhost:8000")
30
-
31
- TEMPERATURE = 0.0
32
- MAX_TOKENS = 300
33
-
34
- TASKS = ["customer_contacts", "sales_records", "employee_records"]
35
-
36
- # ---------------------------------------------------------------------------
37
- # System prompt
38
- # ---------------------------------------------------------------------------
39
- SYSTEM_PROMPT = textwrap.dedent("""\
40
- You are an expert data quality analyst. You are given a dataset with known issues.
41
- Your job is to find and fix ALL data quality problems efficiently.
42
-
43
- Available commands (respond with EXACTLY ONE command per turn, no explanation):
44
- inspect("column_name") — View column statistics and issues
45
- fix(row_index, "column_name", "value") — Fix a specific cell value
46
- delete(row_index) — Remove a duplicate/invalid row
47
- submit() — Finalize and get your score
48
-
49
- STRATEGY (follow this order strictly):
50
- 1. You will first receive inspection results for all columns — read them carefully
51
- 2. Look at the data table and identify ALL issues based on the inspection hints
52
- 3. Fix ALL issues using fix() commands, one at a time
53
- 4. Delete duplicate rows LAST (after all fixes), from highest row index to lowest
54
- (this avoids row index shifting problems)
55
- 5. Only call submit() when you believe ALL issues are fixed
56
-
57
- VALIDATION RULES:
58
- - Emails: must match user@domain.tld (no [at], no @@, no spaces)
59
- - Phone numbers: digits, dashes, parens, spaces only; at least 10 digits
60
- - Dates: YYYY-MM-DD format (e.g., 2024-03-25), must be valid calendar date
61
- - Empty/missing values: provide a reasonable non-empty value matching context
62
- - Negative numbers (quantity/price): make them positive (use absolute value)
63
- - Price/salary outliers: fix to a reasonable value within the stated range
64
- - Inconsistent region/department names: use the EXACT canonical form from the task description
65
- - Excess whitespace: trim leading/trailing spaces, collapse double spaces to single
66
- - Manager IDs: must reference an existing employee emp_id in the dataset
67
- - Performance scores: must be within 0.0-10.0
68
- - Salaries: must be within $20,000-$500,000
69
- - Termination dates: must be AFTER hire date, or leave empty if employee is active
70
- - Duplicate rows: rows that are exact or near-exact copies — delete the LATER one
71
-
72
- IMPORTANT: After deleting a row, all subsequent row indices shift down by 1.
73
- Always delete from highest index to lowest to avoid this issue.
74
-
75
- Respond with ONLY the command. No explanation, no markdown, no extra text.
76
- """)
77
-
78
-
79
- # ---------------------------------------------------------------------------
80
- # HTTP helpers (direct REST calls to the environment server)
81
- # ---------------------------------------------------------------------------
82
- import requests
83
-
84
-
85
- def env_reset(task_id: str) -> Dict[str, Any]:
86
- """Reset the environment with a specific task."""
87
- resp = requests.post(
88
- f"{ENV_URL}/reset",
89
- json={"task_id": task_id},
90
- timeout=30,
91
- )
92
- resp.raise_for_status()
93
- return resp.json()
94
-
95
-
96
- def env_step(command: str) -> Dict[str, Any]:
97
- """Execute a command in the environment."""
98
- resp = requests.post(
99
- f"{ENV_URL}/step",
100
- json={"action": {"command": command}},
101
- timeout=30,
102
- )
103
- resp.raise_for_status()
104
- return resp.json()
105
-
106
-
107
- # ---------------------------------------------------------------------------
108
- # Observation formatting
109
- # ---------------------------------------------------------------------------
110
- def format_observation(obs: Dict[str, Any], step_num: int) -> str:
111
- """Format an observation dict as a user message for the LLM."""
112
- parts = []
113
- parts.append(f"Step {step_num} | Task: {obs.get('task_id', '?')} ({obs.get('difficulty', '?')})")
114
- parts.append(f"Issues fixed: {obs.get('issues_fixed', 0)}/{obs.get('total_issues', 0)} | Score: {obs.get('current_score', 0.0):.4f}")
115
- parts.append(f"Actions remaining: {obs.get('actions_remaining', 0)}")
116
- parts.append("")
117
-
118
- feedback = obs.get("feedback", "")
119
- if feedback:
120
- parts.append(f"Last action result: {feedback}")
121
- parts.append("")
122
-
123
- parts.append("Task description:")
124
- parts.append(obs.get("task_description", ""))
125
- parts.append("")
126
-
127
- parts.append("Column info:")
128
- parts.append(obs.get("column_info", ""))
129
- parts.append("")
130
-
131
- parts.append("Current data:")
132
- parts.append(obs.get("data_preview", ""))
133
-
134
- return "\n".join(parts)
135
-
136
-
137
- # ---------------------------------------------------------------------------
138
- # Action extraction
139
- # ---------------------------------------------------------------------------
140
- ACTION_RE = re.compile(r"(inspect|fix|delete|submit)\s*\(", re.IGNORECASE)
141
-
142
-
143
- def extract_action(response_text: str) -> str:
144
- """Extract a valid action from the LLM response."""
145
- if not response_text:
146
- return "submit()"
147
-
148
- # Try each line
149
- for line in response_text.strip().splitlines():
150
- line = line.strip()
151
- if not line:
152
- continue
153
- # Strip markdown code fences
154
- line = re.sub(r"^```\w*\s*", "", line)
155
- line = re.sub(r"\s*```$", "", line)
156
- # Strip "action:" prefix
157
- line = re.sub(r"^(?:action|next action)\s*[:\-]\s*", "", line, flags=re.IGNORECASE)
158
- if ACTION_RE.search(line):
159
- m = ACTION_RE.search(line)
160
- start = m.start()
161
- depth = 0
162
- for i in range(start, len(line)):
163
- if line[i] == "(":
164
- depth += 1
165
- elif line[i] == ")":
166
- depth -= 1
167
- if depth == 0:
168
- return line[start : i + 1]
169
- return line[start:] + ")"
170
-
171
- return "submit()"
172
-
173
-
174
- # ---------------------------------------------------------------------------
175
- # Main
176
- # ---------------------------------------------------------------------------
177
- def main() -> None:
178
- client = OpenAI(base_url=API_BASE_URL, api_key=API_KEY)
179
- results = {}
180
-
181
- for task_id in TASKS:
182
- print(f"\n{'=' * 60}")
183
- print(f"Task: {task_id}")
184
- print(f"{'=' * 60}")
185
-
186
- obs = env_reset(task_id)
187
- if "observation" in obs:
188
- obs = obs["observation"]
189
-
190
- done = obs.get("done", False)
191
- messages = [{"role": "system", "content": SYSTEM_PROMPT}]
192
-
193
- # --- Phase 1: Auto-inspect all columns ---
194
- columns = []
195
- col_info = obs.get("column_info", "")
196
- for line in col_info.strip().splitlines():
197
- line = line.strip()
198
- if ":" in line:
199
- col_name = line.split(":")[0].strip()
200
- if col_name:
201
- columns.append(col_name)
202
-
203
- inspection_results = []
204
- step_num = 0
205
- for col in columns:
206
- if done:
207
- break
208
- step_num += 1
209
- cmd = f'inspect("{col}")'
210
- print(f" Step {step_num}: {cmd}")
211
- obs = env_step(cmd)
212
- if "observation" in obs:
213
- obs = obs["observation"]
214
- done = obs.get("done", False)
215
- feedback = obs.get("feedback", "")
216
- inspection_results.append(f"[{col}]: {feedback}")
217
-
218
- if done:
219
- score = obs.get("current_score", 0.0)
220
- print(f" Done during inspection! Score: {score:.4f}")
221
- results[task_id] = score
222
- continue
223
-
224
- # Build a summary of all inspections for the LLM
225
- inspection_summary = "\n\n".join(inspection_results)
226
- initial_context = format_observation(obs, step_num)
227
- initial_context += f"\n\n--- INSPECTION SUMMARY (all columns) ---\n{inspection_summary}"
228
- initial_context += "\n\n--- NOW FIX ALL ISSUES. Do fixes first, then deletions (highest index first). ---"
229
-
230
- messages.append({"role": "user", "content": initial_context})
231
-
232
- # --- Phase 2: LLM-driven fixes ---
233
- while not done:
234
- step_num += 1
235
-
236
- # Keep conversation manageable — but preserve system + initial context
237
- if len(messages) > 40:
238
- messages = [messages[0], messages[1]] + messages[-36:]
239
-
240
- try:
241
- completion = client.chat.completions.create(
242
- model=MODEL_NAME,
243
- messages=messages,
244
- temperature=TEMPERATURE,
245
- max_tokens=MAX_TOKENS,
246
- stream=False,
247
- )
248
- response_text = completion.choices[0].message.content or ""
249
- except Exception as exc:
250
- print(f" LLM error: {exc}. Using submit().")
251
- response_text = "submit()"
252
-
253
- action = extract_action(response_text)
254
- messages.append({"role": "assistant", "content": action})
255
- print(f" Step {step_num}: {action}")
256
-
257
- obs = env_step(action)
258
- if "observation" in obs:
259
- obs = obs["observation"]
260
-
261
- done = obs.get("done", False)
262
- score = obs.get("current_score", 0.0)
263
-
264
- if done:
265
- print(f" Done! Score: {score:.4f}")
266
- results[task_id] = score
267
- else:
268
- # Send updated observation as next user message
269
- user_msg = format_observation(obs, step_num)
270
- messages.append({"role": "user", "content": user_msg})
271
-
272
- print(f" Final score for {task_id}: {results.get(task_id, 0.0):.4f}")
273
-
274
- print(f"\n{'=' * 60}")
275
- print("RESULTS SUMMARY")
276
- print(f"{'=' * 60}")
277
- for task_id, score in results.items():
278
- print(f" {task_id}: {score:.4f}")
279
- avg = sum(results.values()) / len(results) if results else 0.0
280
- print(f" Average: {avg:.4f}")
281
-
282
-
283
- if __name__ == "__main__":
284
- main()
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
data_clean_env/server/app.py CHANGED
@@ -1,9 +1,10 @@
1
  """FastAPI application for the DataClean Environment."""
2
 
 
 
3
  from fastapi import FastAPI, Body
4
- from fastapi.responses import JSONResponse
5
  from pydantic import BaseModel
6
- from typing import Any, Dict, List, Optional
7
 
8
  try:
9
  from .environment import DataCleanEnvironment
@@ -16,8 +17,9 @@ except ImportError:
16
 
17
  app = FastAPI(title="DataCleanEnv", version="1.0.0")
18
 
19
- # Shared stateful environment instance
20
  _env = DataCleanEnvironment()
 
21
 
22
 
23
  # ---------------------------------------------------------------------------
@@ -49,14 +51,16 @@ async def health():
49
 
50
  @app.post("/reset")
51
  async def reset(request: ResetRequest = Body(default_factory=ResetRequest)):
52
- obs = _env.reset(seed=request.seed, episode_id=request.episode_id, task_id=request.task_id)
 
53
  return {"observation": _obs_dict(obs), "reward": None, "done": False}
54
 
55
 
56
  @app.post("/step")
57
  async def step(request: StepRequest):
58
  action = DataCleanAction(**request.action)
59
- obs = _env.step(action)
 
60
  return {"observation": _obs_dict(obs), "reward": obs.reward, "done": obs.done}
61
 
62
 
 
1
  """FastAPI application for the DataClean Environment."""
2
 
3
+ import asyncio
4
+
5
  from fastapi import FastAPI, Body
 
6
  from pydantic import BaseModel
7
+ from typing import Any, Dict, Optional
8
 
9
  try:
10
  from .environment import DataCleanEnvironment
 
17
 
18
  app = FastAPI(title="DataCleanEnv", version="1.0.0")
19
 
20
+ # Shared stateful environment instance with lock for concurrent safety
21
  _env = DataCleanEnvironment()
22
+ _env_lock = asyncio.Lock()
23
 
24
 
25
  # ---------------------------------------------------------------------------
 
51
 
52
  @app.post("/reset")
53
  async def reset(request: ResetRequest = Body(default_factory=ResetRequest)):
54
+ async with _env_lock:
55
+ obs = _env.reset(seed=request.seed, episode_id=request.episode_id, task_id=request.task_id)
56
  return {"observation": _obs_dict(obs), "reward": None, "done": False}
57
 
58
 
59
  @app.post("/step")
60
  async def step(request: StepRequest):
61
  action = DataCleanAction(**request.action)
62
+ async with _env_lock:
63
+ obs = _env.step(action)
64
  return {"observation": _obs_dict(obs), "reward": obs.reward, "done": obs.done}
65
 
66
 
data_clean_env/server/environment.py CHANGED
@@ -288,7 +288,7 @@ class DataCleanEnvironment(Environment):
288
  # Agent is modifying a cell that had no known issue — damage
289
  self._current_data[row_idx][col] = new_value
290
  self._damaged_cells += 1
291
- self._score = max(0.0, self._score - 0.05)
292
  return (
293
  f"Warning: Row {row_idx}, column '{col}' had no known issue. "
294
  f"Original value '{old_value}' was overwritten. Penalty applied (-0.05)."
@@ -313,7 +313,7 @@ class DataCleanEnvironment(Environment):
313
  reward_delta += 1.0 / len(self._task.issues)
314
  fixed_any = True
315
 
316
- self._score = min(1.0, max(0.0, self._score + reward_delta))
317
 
318
  if fixed_any:
319
  return (
@@ -337,7 +337,6 @@ class DataCleanEnvironment(Environment):
337
  deleted_row = self._current_data[row_idx]
338
 
339
  # Check if this row is a known duplicate
340
- reward_delta = 0.0
341
  is_duplicate = False
342
  for issue in self._task.issues:
343
  if issue.issue_type != "duplicate_row":
@@ -347,21 +346,18 @@ class DataCleanEnvironment(Environment):
347
  if self._issue_status.get(issue.issue_id, False):
348
  continue
349
 
350
- # The agent is deleting the row that matches the duplicate issue
351
  self._issue_status[issue.issue_id] = True
352
- reward_delta += 1.0 / len(self._task.issues)
353
  is_duplicate = True
354
 
355
  if not is_duplicate:
356
  # Deleting a non-duplicate row — penalty
357
  self._damaged_cells += 1
358
- reward_delta = -0.05
359
 
360
  # Actually remove the row
361
  self._current_data.pop(row_idx)
362
  self._row_index_map.pop(row_idx)
363
 
364
- self._score = min(1.0, max(0.0, self._score + reward_delta))
365
 
366
  if is_duplicate:
367
  return (
@@ -380,7 +376,8 @@ class DataCleanEnvironment(Environment):
380
  def _check_issue_resolved(self, issue: Issue) -> bool:
381
  """Check if an issue is resolved in the current data."""
382
  if issue.issue_type == "duplicate_row":
383
- return validate_row_deleted(self._current_data, issue.original_row_data)
 
384
 
385
  if issue.issue_type == "temporal_inconsistency":
386
  # Find the row by original index
@@ -425,6 +422,17 @@ class DataCleanEnvironment(Environment):
425
  return new_value
426
  return new_value
427
 
 
 
 
 
 
 
 
 
 
 
 
428
  def _compute_final_score(self) -> None:
429
  """Recompute score based on all current issue states."""
430
  total = len(self._task.issues)
 
288
  # Agent is modifying a cell that had no known issue — damage
289
  self._current_data[row_idx][col] = new_value
290
  self._damaged_cells += 1
291
+ self._recompute_score()
292
  return (
293
  f"Warning: Row {row_idx}, column '{col}' had no known issue. "
294
  f"Original value '{old_value}' was overwritten. Penalty applied (-0.05)."
 
313
  reward_delta += 1.0 / len(self._task.issues)
314
  fixed_any = True
315
 
316
+ self._recompute_score()
317
 
318
  if fixed_any:
319
  return (
 
337
  deleted_row = self._current_data[row_idx]
338
 
339
  # Check if this row is a known duplicate
 
340
  is_duplicate = False
341
  for issue in self._task.issues:
342
  if issue.issue_type != "duplicate_row":
 
346
  if self._issue_status.get(issue.issue_id, False):
347
  continue
348
 
 
349
  self._issue_status[issue.issue_id] = True
 
350
  is_duplicate = True
351
 
352
  if not is_duplicate:
353
  # Deleting a non-duplicate row — penalty
354
  self._damaged_cells += 1
 
355
 
356
  # Actually remove the row
357
  self._current_data.pop(row_idx)
358
  self._row_index_map.pop(row_idx)
359
 
360
+ self._recompute_score()
361
 
362
  if is_duplicate:
363
  return (
 
376
  def _check_issue_resolved(self, issue: Issue) -> bool:
377
  """Check if an issue is resolved in the current data."""
378
  if issue.issue_type == "duplicate_row":
379
+ # Row is resolved if it was deleted (no longer in index map)
380
+ return self._find_current_row(issue.row) is None
381
 
382
  if issue.issue_type == "temporal_inconsistency":
383
  # Find the row by original index
 
422
  return new_value
423
  return new_value
424
 
425
+ def _recompute_score(self) -> None:
426
+ """Recompute running score based on current issue states and damage."""
427
+ total = len(self._task.issues)
428
+ if total == 0:
429
+ self._score = 1.0
430
+ return
431
+ resolved = sum(1 for v in self._issue_status.values() if v)
432
+ self._score = resolved / total
433
+ self._score = max(0.0, self._score - self._damaged_cells * 0.05)
434
+ self._score = min(1.0, self._score)
435
+
436
  def _compute_final_score(self) -> None:
437
  """Recompute score based on all current issue states."""
438
  total = len(self._task.issues)