Skip to content

fix(work): keep marker-like hunk lines in the attempt diff canvas - #275

Merged
Lucas1479 merged 2 commits into
Code-Amadeus:mainfrom
NopeCharon:fix/unified-diff-hunk-markers
Nov 1, 2026
Merged

Lucas1479 merged 2 commits into
Code-Amadeus:mainfrom
NopeCharon:fix/unified-diff-hunk-markers

Conversation

@NopeCharon

Copy link
Copy Markdown
Contributor

What and why

parse_unified_diff checked for --- / +++ file-header markers before it looked at hunk content. Git prefixes removed lines with - and added lines with +, so removing a line that starts with -- (an SQL, Lua or Haskell comment) or adding one that starts with ++ produces --- ... / +++ ... inside a hunk. These lines were treated as file headers. The attempt diff canvas then renamed the file to the line text, dropped the line, and under-counted additions and deletions (details in #274).

Hunk headers already declare how many old and new lines follow, and the parser already read those counts into oldCount / newCount. The fix tracks the remaining old and new counts as hunk lines are consumed. A --- / +++ line is accepted as a file header only after the current hunk's declared lines have all been consumed. The counts are decremented before the max_lines cap, so a truncated hunk still consumes its body and the next file's headers still parse.

Fixes #274

Linked Issue for product-semantic or public-contract changes: none (routine fix)

Change class

  • Routine fix, documentation, test, maintenance, or presentation-only UI
  • Product-semantic or public-contract change discussed in the linked Issue
  • Isolated, default-off experiment

Owning layer: server/unified_diff.py, the parser that turns git diff output into the provider-neutral diff canvas shape.

User-visible effect, or none: the Work attempt diff canvas keeps the correct file path and shows removed -- ... / added ++ ... lines with correct line numbers and +/- counts.

Compatibility or migration impact, or none: none. The output shape is unchanged.

Evidence

Commands and manual journeys run:

  • python -m pytest -q tests/test_unified_diff_canvas.py tests/test_work_export_integration.py: 28 passed (Linux, uv sync --locked --extra dev).

  • The new test test_parse_unified_diff_keeps_marker_like_content_inside_hunks fails on main and passes with this change.

  • python -m ruff check on the changed files: passed.

  • Parsed real git diff output from a scratch repository, where a line -- legacy index was replaced with ++ counter and a second file was also edited. Both files keep their paths and counts, including with max_lines=1.

  • tools/run_tests.py on Linux: the only failures are in Windows/environment-dependent suites. For test_work_ledger_coordinator.py, test_auip_work_recovery.py and test_backend_cleanup.py the failing tests are identical with and without this change.

  • Relevant Python tests pass

  • CPU/model-less baseline remains supported

  • Electron npm run build passes when Electron code changed

  • Dependency audit passes when dependencies changed

  • Before/after screenshots are attached for visible UI changes

  • Documentation/examples are updated for changed settings or contracts

Final check

  • This PR addresses one coherent problem without unrelated cleanup
  • It does not add a speculative API, fallback, or compatibility path
  • No secrets, local state, model weights, voice material, or restricted assets are included
  • Third-party notices and provenance are preserved

parse_unified_diff checked for "--- " and "+++ " file-header markers
before it looked at hunk content. Git prefixes every removed line with
"-" and every added line with "+", so removing a line that starts with
"-- " (an SQL, Lua or Haskell comment) or adding one that starts with
"++ " produces "--- ..." / "+++ ..." inside a hunk. The parser treated
those lines as file headers: the canvas renamed the file to the line
text, dropped the line, and under-counted additions and deletions.

Hunk headers already declare how many old and new lines follow, and the
parser stored those counts. Track the remaining old/new counts while
consuming hunk lines, and only accept "--- " / "+++ " as file headers
once the current hunk's declared lines have been consumed. The counts
are decremented before the max_lines cap, so truncated hunks still
consume their declared body and the next file's headers parse normally.

Add a regression test with a removed "-- " line and an added "++ " line
followed by a second file, asserting the original path is kept, both
lines render with correct line numbers, and the second file still
parses.

Fixes Code-Amadeus#274
@NopeCharon
NopeCharon requested a review from Lucas1479 as a code owner October 31, 2026 03:43
Lucas1479 added a commit that referenced this pull request Nov 1, 2026
## What and why

The Python dependency audit blocks #275 because main locks `pyasn1
0.6.4`, which is affected by CVE-2026-119194, CVE-2026-119195,
CVE-2026-119196 and CVE-2026-119197 ([failed
audit](https://github.com/Code-Amadeus/Amadeus/actions/runs/41165567440/job/124472772428)).

Update only the `pyasn1` package entry in `uv.lock` to [upstream's
patched 0.6.5
release](https://github.com/pyasn1/pyasn1/releases/tag/v0.6.5). It
remains a transitive dependency of `google-genai -> google-auth ->
pyasn1-modules`; 0.6.5 satisfies the existing `>=0.6.1,<0.7.0`
constraint. Both artifact hashes match the upstream release assets. No
new top-level dependency, audit exception, or CI change is introduced.

This separate prerequisite keeps #275 focused on its parser fix. Linked
Issue for product-semantic or public-contract changes: none (routine
dependency maintenance).

## Change class

- [x] Routine fix, documentation, test, maintenance, or
presentation-only UI

Owning layer: the canonical Python dependency lock.

User-visible effect: restore the dependency audit prerequisite for
normal CI and merge validation.

Compatibility or migration impact: none expected; sync the updated
lockfile through the existing installation procedure.

## Evidence

Fresh core/dev environment on Windows, CPython 3.12.10, uv 0.12.8:

- `uv lock --upgrade-package pyasn1==0.6.5` and `uv sync --locked
--extra dev`: passed.
- Exact CI command, `uv run --locked --no-sync python -m pip_audit
--local --progress-spinner off --ignore-vuln PYSEC-2026-1845`: passed,
no known vulnerabilities and 2 existing ignores.
- `uv lock --check --offline` and `uv run --locked --no-sync python
tools/verify_python_environment.py --profile ci`: passed.
- `uv run --locked --no-sync python -X utf8 -m pytest
tests/test_gemini_client.py tests/test_unified_diff_canvas.py
tests/test_work_export_integration.py -q`: **29 passed**.
- Offline synthetic compatibility checks: PKCS#8 DER decode/re-encode
with the real `pyasn1-modules` schema, plus Google Auth RSA signing,
valid-signature verification and tampered-message rejection: passed. The
Google Auth check exercises its installed cryptography backend; it does
not claim coverage of the optional `rsa` backend.
- `uv run --locked --no-sync python -m ruff check .` and `git diff
--check`: passed.
- Structural TOML comparison against base
`65cea5fff216fda92ce29a9d6fd91a405b10ed27`: the only changed package
entry is `pyasn1 0.6.4 -> 0.6.5`; all other lock metadata and package
entries are identical.

- [x] Relevant Python tests pass
- [x] CPU/model-less baseline remains supported
- [x] Dependency audit passes

All **18 hosted checks across 8 workflows passed** on head
`aeab62eaf4f2bc158f9338f65e7bb598c60a5a99`, including [the original
audit, source/packaged desktop smoke, and catalog
publication/recovery](https://github.com/Code-Amadeus/Amadeus/actions/runs/41294772512).
No retries were needed. An isolated composition of the exact #275
parser/test blobs with the repaired dependency also passed **30 targeted
tests**. No live provider calls were made. No Electron code, UI,
settings or public contracts changed.

## Final check

- [x] One coherent problem without unrelated cleanup
- [x] No speculative API, fallback, or compatibility path
- [x] No secrets, local state, model weights, voice material, or
restricted assets
- [x] Third-party notices and provenance preserved

---

中文:此 PR 仅将 `uv.lock` 中的传递依赖 `pyasn1` 从 0.6.4 更新到修复上述四个漏洞的 0.6.5,解除 #275
的 CI 审计阻塞。未修改审计规则或其他依赖。全新 Windows 开发环境中的原样 CI 审计、29 个相关测试、锁文件/环境一致性、离线
ASN.1 与 Google Auth 兼容性检查及 Ruff 均通过;提交 `aeab62ea` 的 8 个工作流、18
项托管检查现已全部通过,无重试;原先被审计挡住的源码/打包应用 smoke 和目录发布/恢复验证也已通过。与 #275 已审查代码组合后的 30
个针对性测试同样通过。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Work diff canvas treats removed '-- ' and added '++ ' lines as file headers

2 participants