Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
46 changes: 45 additions & 1 deletion src/python/marcut/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
33 changes: 31 additions & 2 deletions tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": []}'
Expand Down
Loading