diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 7054057..7d0f0e7 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -38,3 +38,7 @@ Each of these has a design-spike doc under `docs/design/` — read the linked do - **Multi-Model Orchestration Workflow**: Fast first-pass model with escalation to a larger model for low-confidence chunks. See `docs/design/multi_model_orchestration.md`. - **Incremental Track-Changes Support**: Diff-only redaction of newly-added paragraphs in a revised document. See `docs/design/incremental_track_changes_redaction.md` — flags a real under-redaction risk that needs a mitigation plan before implementation. - **Automated "Redaction Rationale" Reporting**: LLM-generated plain-English explanations in the audit log. See `docs/design/redaction_rationale_reporting.md` (covers how to label LLM-generated rationale as inference, not verified fact). + +## 4. CI / Infrastructure + +- **Self-Hosted GitHub Actions Runner** ([PR #34](https://github.com/LegalMarc/Marcut/pull/34), open, deferred): Points `ci.yml`, `macos-build-verify.yml`, and `macos-full-e2e.yml` at a local `[self-hosted, marcut-local]` runner instead of GitHub-hosted `macos-14`. Deferred during the 2026-07-15 release-readiness pass: the nightly/tag E2E red streak that motivated looking at this again turned out to have a deterministic root cause unrelated to runner choice (a `//`-comment-stripping regex in `parse_llm_response()` corrupting any LLM-extracted entity containing a URL — fixed on `fix/json-comment-strip-url-corruption`), so switching runners isn't needed to restore green CI. Revisit only if GitHub-hosted runner cost/queue-time/resource limits become a concrete problem — the security tradeoff noted in the PR (self-hosted runners execute arbitrary code from workflow-triggering events on the host machine) means it shouldn't be merged just because it's sitting there. diff --git a/src/python/marcut/model.py b/src/python/marcut/model.py index 2a65fbc..35ab3fc 100644 --- a/src/python/marcut/model.py +++ b/src/python/marcut/model.py @@ -143,6 +143,50 @@ def _repair_unbalanced_json(json_str: str) -> Optional[str]: return repaired +def _strip_line_comments_outside_strings(json_str: str) -> str: + """ + Strip `//`-style line comments that appear outside JSON string literals. + + A naive `re.sub(r'//.*$', '', ...)` would also truncate legitimate string + values that happen to contain `//` -- most commonly a URL entity like + `"https://legal.example"` extracted by the LLM -- corrupting otherwise- + valid JSON. This walks the string tracking whether we're inside a string + literal (respecting `\\"` escapes) and only treats `//` as a comment + start when outside one. + """ + out: List[str] = [] + in_string = False + escape = False + i = 0 + n = len(json_str) + while i < n: + ch = json_str[i] + if in_string: + out.append(ch) + if escape: + escape = False + elif ch == '\\': + escape = True + elif ch == '"': + in_string = False + i += 1 + continue + if ch == '"': + in_string = True + out.append(ch) + i += 1 + continue + if ch == '/' and i + 1 < n and json_str[i + 1] == '/': + newline_pos = json_str.find('\n', i) + if newline_pos == -1: + break + i = newline_pos + continue + out.append(ch) + i += 1 + return ''.join(out) + + def parse_llm_response(response_text: str) -> Dict[str, Any]: """ Sanitize and parse an LLM response, raising json.JSONDecodeError when invalid. @@ -166,7 +210,7 @@ def parse_llm_response(response_text: str) -> Dict[str, Any]: else: json_str = cleaned[start:end] - json_str = re.sub(r'(?m)//.*$', '', json_str) + json_str = _strip_line_comments_outside_strings(json_str) json_str = re.sub(r',\s*(\]|\})', r'\1', json_str) try: diff --git a/tests/test_model.py b/tests/test_model.py index 36ea155..38c2ca4 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -84,9 +84,38 @@ def test_parse_json_with_comments(self): {"text": "John", "type": "NAME"} // This is a name ]}''' result = parse_llm_response(response) - + assert 'entities' in result - + + def test_parse_json_with_url_entity_containing_double_slash(self): + """A `//`-containing string value (e.g. a URL entity) must survive + comment-stripping intact -- a naive `//.*$` regex would truncate the + string mid-value and corrupt the JSON (root cause of the nightly E2E + failure streak: the LLM extracting "https://legal.example" as an + entity).""" + response = json.dumps({ + "entities": [ + {"text": "alice@example.com", "type": "NAME"}, + {"text": "https://legal.example", "type": "ORG"}, + ] + }) + result = parse_llm_response(response) + + assert len(result['entities']) == 2 + assert result['entities'][1]['text'] == 'https://legal.example' + + def test_parse_json_with_comment_after_url_entity(self): + """A genuine trailing `//` comment after a URL-containing entity line + must still be stripped, without corrupting the URL itself.""" + response = '''{"entities": [ + {"text": "https://legal.example", "type": "ORG"}, // a URL + {"text": "Jane Doe", "type": "NAME"} + ]}''' + result = parse_llm_response(response) + + assert result['entities'][0]['text'] == 'https://legal.example' + assert result['entities'][1]['text'] == 'Jane Doe' + def test_parse_empty_entities(self): """Test parsing response with no entities.""" response = '{"entities": []}'