Skip to content

Commit 0aed5f1

Browse files
committed
Support file uploads via response payload and prioritize over params["files"]
Adds `_resolve_submission` to handle responses containing code and file specifications in `"code"` and `"files"` keys. Updates `evaluation.py` to normalize and prioritize file sources from the response payload over `params["files"]`. Enhances tests to validate this logic. Updates documentation to reflect the new behavior.
1 parent 208bf57 commit 0aed5f1

3 files changed

Lines changed: 153 additions & 6 deletions

File tree

CLAUDE.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ All source lives in `evaluation_function/`:
1717
### Evaluation pipeline (`evaluation.py`)
1818

1919
1. Run AST security check on student code
20-
2. If `params["files"]` is set, download the listed S3 objects once into a per-request working directory (see `s3_files.py`), used as the subprocess `cwd` for every run in this request
20+
2. Resolve the submission into `(code, file_specs)` via `_resolve_submission`: the response may be a bare code string, or a `{"code", "files"}` object (or JSON string of one) as sent by the LF web client's upload widget. If `file_specs` (from `response["files"]`, else `params["files"]`) is non-empty, download the listed objects once into a per-request working directory (see `s3_files.py`), used as the subprocess `cwd` for every run in this request
2121
3. Dispatch by `params["mode"]` (required):
2222
- **`demo`**: execute code with no stdin; return stdout/plots as `output` feedback (no pass/fail)
2323
- **`io_test`**: for each test in `params["tests"]`, execute with `test["input"]` as stdin and compare stdout against `test["expected_output"]`; upload matplotlib plots on pass or fail
@@ -108,6 +108,17 @@ All source lives in `evaluation_function/`:
108108
{"url": "https://.../helper.py?X-Amz-Signature=...", "name": "helper.py"},
109109
]
110110
}
111+
112+
# files in the response payload (how the LF web client sends uploads)
113+
# When the response area has a file-upload widget, the client delivers the
114+
# submission as {"code": ..., "files": [...]} (sometimes as a JSON string of
115+
# that object), with each file entry itself possibly a JSON string.
116+
# evaluation_function unpacks this: response["code"] becomes the student
117+
# code, response["files"] becomes the file list. Files in the response take
118+
# precedence over params["files"], which stays as a fallback. Entry shape is
119+
# the same {"url", "name"} as params["files"].
120+
{"code": "print(open('data.csv').read())",
121+
"files": [{"url": "https://.../data.csv?...", "name": "data.csv"}]}
111122
```
112123

113124
### Security model (`preview.py`)

evaluation_function/evaluation.py

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,37 +322,86 @@ def _evaluate_unit(response: str, test_code: str, result: Result, files_dir: str
322322
return result
323323

324324

325+
def _coerce_file_specs(raw: Any) -> list:
326+
"""Normalise a raw files value into a list of {url, name} dicts.
327+
328+
Entries may already be dicts, or JSON-encoded strings — the LF web
329+
client currently serialises each upload entry to a string.
330+
"""
331+
if not isinstance(raw, (list, tuple)):
332+
return []
333+
specs = []
334+
for entry in raw:
335+
if isinstance(entry, str):
336+
try:
337+
entry = json.loads(entry)
338+
except (ValueError, TypeError):
339+
continue
340+
if isinstance(entry, dict):
341+
specs.append(entry)
342+
return specs
343+
344+
345+
def _resolve_submission(response: Any, params: Params) -> tuple[str, list]:
346+
"""Split the submission into (code, file_specs).
347+
348+
When file upload is enabled, the LF web client delivers the response
349+
payload as {"code": ..., "files": [...]} (sometimes as a JSON string of
350+
that object) rather than a bare code string. Files listed in the
351+
response take precedence; params["files"] is the fallback.
352+
"""
353+
payload = response
354+
if isinstance(payload, str):
355+
try:
356+
parsed = json.loads(payload)
357+
except (ValueError, TypeError):
358+
parsed = None
359+
if isinstance(parsed, dict) and ("code" in parsed or "files" in parsed):
360+
payload = parsed
361+
362+
if isinstance(payload, dict):
363+
code = payload.get("code") or ""
364+
response_files = _coerce_file_specs(payload.get("files"))
365+
else:
366+
code = payload if isinstance(payload, str) else str(payload)
367+
response_files = []
368+
369+
file_specs = response_files or _coerce_file_specs(params.get("files"))
370+
return str(code), file_specs
371+
372+
325373
def evaluation_function(response: Any, answer: Any, params: Params) -> Result:
326374
result = Result()
327375
mode = params.get("mode")
328376
if mode not in ("demo", "io_test", "unit_test"):
329377
result.add_feedback("error", f"Unknown or missing mode: {mode!r}. Expected 'demo', 'io_test', or 'unit_test'.")
330378
return result
331379

380+
code, file_specs = _resolve_submission(response, params)
381+
332382
files_dir = None
333383
try:
334384
file_warnings: list[str] = []
335-
file_specs = params.get("files")
336385
if file_specs:
337386
files_dir = tempfile.mkdtemp()
338387
file_warnings = download_files(file_specs, files_dir)
339388

340389
if mode == "demo":
341-
result = _evaluate_demo(str(response), result, files_dir)
390+
result = _evaluate_demo(code, result, files_dir)
342391
elif mode == "io_test":
343392
ans = str(answer) if params.get("use_answer_as_expected_output") else ""
344-
result = _evaluate_io(str(response), params.get("tests", []), result, answer=ans, files_dir=files_dir)
393+
result = _evaluate_io(code, params.get("tests", []), result, answer=ans, files_dir=files_dir)
345394
else:
346395
test_code = str(answer) if params.get("use_answer_as_test_code") else params.get("test_code", "")
347-
result = _evaluate_unit(str(response), test_code, result, files_dir=files_dir)
396+
result = _evaluate_unit(code, test_code, result, files_dir=files_dir)
348397

349398
for warning in file_warnings:
350399
result.add_feedback("error", warning)
351400

352401
pep8_param = params.get("pep8_feedback")
353402
if pep8_param:
354403
select = pep8_param if isinstance(pep8_param, list) else _PEP8_SELECT
355-
violations = _check_pep8(str(response), select)
404+
violations = _check_pep8(code, select)
356405
if violations:
357406
body = "Style suggestions (PEP8):\n" + "\n".join(f"- {v}" for v in violations)
358407
else:

evaluation_function/evaluation_test.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import json
12
import os
23
import tempfile
34
import unittest
@@ -392,6 +393,92 @@ def test_legacy_filename_key_does_not_crash(self):
392393
self.assertIn("missing", result["feedback"].lower())
393394

394395

396+
class TestFilesInResponsePayload(unittest.TestCase):
397+
"""The LF web client delivers uploads inside the response payload as
398+
{"code": ..., "files": [...]} rather than in params["files"]."""
399+
400+
@patch("evaluation_function.evaluation.download_files")
401+
def test_response_dict_with_code_and_files(self, mock_download):
402+
mock_download.side_effect = _stub_download({"data.csv": "1,2,3"})
403+
response = {
404+
"code": "print(open('data.csv').read())",
405+
"files": [{"url": "https://example.com/k", "name": "data.csv"}],
406+
}
407+
result = evaluation_function(response, None, {"mode": "demo"}).to_dict()
408+
409+
self.assertIn("1,2,3", result["feedback"])
410+
mock_download.assert_called_once()
411+
passed_specs = mock_download.call_args[0][0]
412+
self.assertEqual(passed_specs, [{"url": "https://example.com/k", "name": "data.csv"}])
413+
414+
@patch("evaluation_function.evaluation.download_files")
415+
def test_response_dict_file_entries_are_json_strings(self, mock_download):
416+
mock_download.side_effect = _stub_download({"data.csv": "42"})
417+
response = {
418+
"code": "print(open('data.csv').read())",
419+
"files": [json.dumps({"url": "https://example.com/k", "name": "data.csv"})],
420+
}
421+
result = evaluation_function(response, None, {"mode": "demo"}).to_dict()
422+
423+
self.assertIn("42", result["feedback"])
424+
passed_specs = mock_download.call_args[0][0]
425+
self.assertEqual(passed_specs, [{"url": "https://example.com/k", "name": "data.csv"}])
426+
427+
@patch("evaluation_function.evaluation.download_files")
428+
def test_response_is_json_string_of_payload(self, mock_download):
429+
mock_download.side_effect = _stub_download({"data.csv": "7"})
430+
response = json.dumps({
431+
"code": "print(open('data.csv').read())",
432+
"files": [{"url": "https://example.com/k", "name": "data.csv"}],
433+
})
434+
result = evaluation_function(response, None, {"mode": "demo"}).to_dict()
435+
436+
self.assertIn("7", result["feedback"])
437+
mock_download.assert_called_once()
438+
439+
@patch("evaluation_function.evaluation.download_files")
440+
def test_unit_test_mode_reads_files_from_response(self, mock_download):
441+
mock_download.side_effect = _stub_download({"data.csv": "x"})
442+
response = {
443+
"code": "",
444+
"files": [{"url": "https://example.com/k", "name": "data.csv"}],
445+
}
446+
params = {
447+
"mode": "unit_test",
448+
"test_code": "import os\ndef test_present():\n assert os.path.isfile('data.csv')\n",
449+
}
450+
result = evaluation_function(response, None, params).to_dict()
451+
452+
self.assertTrue(result["is_correct"])
453+
self.assertIn("1/1 tests passed", result["feedback"])
454+
455+
@patch("evaluation_function.evaluation.download_files")
456+
def test_plain_string_response_still_uses_params_files(self, mock_download):
457+
mock_download.side_effect = _stub_download({"data.csv": "9"})
458+
params = {"mode": "demo", "files": [{"url": "https://example.com/k", "name": "data.csv"}]}
459+
result = evaluation_function("print(open('data.csv').read())", None, params).to_dict()
460+
461+
self.assertIn("9", result["feedback"])
462+
463+
@patch("evaluation_function.evaluation.download_files")
464+
def test_response_files_take_precedence_over_params_files(self, mock_download):
465+
mock_download.side_effect = _stub_download({"data.csv": "from_response"})
466+
response = {
467+
"code": "print(open('data.csv').read())",
468+
"files": [{"url": "https://example.com/response", "name": "data.csv"}],
469+
}
470+
params = {"mode": "demo", "files": [{"url": "https://example.com/params", "name": "other.csv"}]}
471+
evaluation_function(response, None, params)
472+
473+
passed_specs = mock_download.call_args[0][0]
474+
self.assertEqual(passed_specs, [{"url": "https://example.com/response", "name": "data.csv"}])
475+
476+
def test_response_dict_without_files_no_download(self):
477+
with patch("evaluation_function.evaluation.download_files") as mock_download:
478+
evaluation_function({"code": "print('hi')"}, None, {"mode": "demo"})
479+
mock_download.assert_not_called()
480+
481+
395482
class TestUnexpectedExceptionHandling(unittest.TestCase):
396483

397484
@patch("evaluation_function.evaluation._run_code")

0 commit comments

Comments
 (0)