Phase 5 — Hardening #15

Merged
coding-agent-marvin8 merged 0 commits from refs/pull/15/head into main 2026-06-11 03:45:14 +00:00
coding-agent-marvin8 commented 2026-06-10 21:49:35 +00:00 (Migrated from codeberg.org)

Summary

  • Carried forward staged changes: ty pre-commit hook, uv-pre-commit bump, sorted test imports
  • PR size gate: new WUMING_MAX_DIFF_LINES env var; posts plain PR comment and exits early if diff exceeds the limit (0 = disabled)
  • Hunk size cap: truncate_large_hunks() replaces hunks > 300 lines with a single stub line before sending to agents
  • Approximate token budget: logs len(diff_text) // 4 per agent call via structured logger
  • HTTP retry with exponential backoff: with_retry() helper in retry.py; applied to ForgejoClient and all three backends
  • Structured JSON logging: log.py with JSONFormatter + setup_logging(); all print() calls replaced
  • Quote-based line resolution: "quote" field added to agent JSON schema; diff searched by text to resolve position before falling back to LLM-supplied line number

Closes #14

Test plan

  • uv run ruff format --check .
  • uv run ruff check .
  • uv run ty check .
  • uv run complexipy .
  • uv run tryke test (192 tests passing)
## Summary - Carried forward staged changes: ty pre-commit hook, uv-pre-commit bump, sorted test imports - **PR size gate**: new `WUMING_MAX_DIFF_LINES` env var; posts plain PR comment and exits early if diff exceeds the limit (0 = disabled) - **Hunk size cap**: `truncate_large_hunks()` replaces hunks > 300 lines with a single stub line before sending to agents - **Approximate token budget**: logs `len(diff_text) // 4` per agent call via structured logger - **HTTP retry with exponential backoff**: `with_retry()` helper in `retry.py`; applied to `ForgejoClient` and all three backends - **Structured JSON logging**: `log.py` with `JSONFormatter` + `setup_logging()`; all `print()` calls replaced - **Quote-based line resolution**: `"quote"` field added to agent JSON schema; diff searched by text to resolve position before falling back to LLM-supplied line number Closes #14 ## Test plan - [x] `uv run ruff format --check .` - [x] `uv run ruff check .` - [x] `uv run ty check .` - [x] `uv run complexipy .` - [x] `uv run tryke test` (192 tests passing)
coding-agent-marvin8 commented 2026-06-11 02:43:14 +00:00 (Migrated from codeberg.org)

src/wuming/diff.py line 7

The truncate_large_hunks function replaces an oversized hunk with a stub line. However, when a hunk is truncated…

No action needed: each Hunk's new_start is parsed directly from the @@ -old +new,count @@ diff header by _DiffParser, not computed incrementally from previous hunks. Truncating hunk.lines to a single stub has no effect on any subsequent hunk's new_start. Both build_position_map and find_line_by_quote use each hunk's new_start as an independent starting point.

[`src/wuming/diff.py` line 7](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237171) > The truncate_large_hunks function replaces an oversized hunk with a stub line. However, when a hunk is truncated… No action needed: each `Hunk`'s `new_start` is parsed directly from the `@@ -old +new,count @@` diff header by `_DiffParser`, not computed incrementally from previous hunks. Truncating `hunk.lines` to a single stub has no effect on any subsequent hunk's `new_start`. Both `build_position_map` and `find_line_by_quote` use each hunk's `new_start` as an independent starting point.
coding-agent-marvin8 commented 2026-06-11 02:43:22 +00:00 (Migrated from codeberg.org)

src/wuming/diff.py line 29

The stub line includes the original line count in the message, but line_count is always equal to len(hunk.lines)…

No action needed: len(hunk.lines) counts every line in the raw diff hunk (context, additions, deletions), which is the right signal — it shows the agent how many diff lines it would have had to read. Showing only added lines would understate the size of the change. The stub is informational and has no effect on correctness.

[`src/wuming/diff.py` line 29](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237174) > The stub line includes the original line count in the message, but line_count is always equal to len(hunk.lines)… No action needed: `len(hunk.lines)` counts every line in the raw diff hunk (context, additions, deletions), which is the right signal — it shows the agent how many diff lines it would have had to read. Showing only added lines would understate the size of the change. The stub is informational and has no effect on correctness.
coding-agent-marvin8 commented 2026-06-11 02:43:29 +00:00 (Migrated from codeberg.org)

src/wuming/diff.py line 67

In find_line_by_quote, after iterating through hunks, if no match is found in the matching file diff, it returns None pr…

No action needed: a PR diff contains at most one entry per file path. Once the loop finds file_diff.path == path and exhausts all its hunks without a match, there is nothing further to search. The early return None is intentional.

[`src/wuming/diff.py` line 67](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237177) > In find_line_by_quote, after iterating through hunks, if no match is found in the matching file diff, it returns None pr… No action needed: a PR diff contains at most one entry per file path. Once the loop finds `file_diff.path == path` and exhausts all its hunks without a match, there is nothing further to search. The early `return None` is intentional.
coding-agent-marvin8 commented 2026-06-11 02:43:37 +00:00 (Migrated from codeberg.org)

src/wuming/retry.py line 45

The variable last_exception is assigned in every except block, but on the last attempt…

No action needed: the assert last_exception is not None is only reachable after all attempts are exhausted (at least one exception was caught), so last_exception is guaranteed set at that point. The assert exists to narrow the type from BaseException | None to BaseException for the type checker.

[`src/wuming/retry.py` line 45](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237180) > The variable `last_exception` is assigned in every except block, but on the last attempt… No action needed: the `assert last_exception is not None` is only reachable after all attempts are exhausted (at least one exception was caught), so `last_exception` is guaranteed set at that point. The assert exists to narrow the type from `BaseException | None` to `BaseException` for the type checker.
coding-agent-marvin8 commented 2026-06-11 02:43:44 +00:00 (Migrated from codeberg.org)

src/wuming/retry.py line 50

The exponential backoff delay uses base_delay * (2**attempt), where attempt starts at 0…

The docstring has an off-by-one error. The code uses base_delay * 2 ** attempt where attempt is 0-indexed, giving delays of base_delay * 1, base_delay * 2, base_delay * 4 for the 2nd, 3rd, 4th attempts — that is the intended behaviour. The docstring incorrectly states base_delay * 2 ** (k - 2) (which would give 0.5x on the second attempt); it should read base_delay * 2 ** (k - 1). Will fix the docstring.

[`src/wuming/retry.py` line 50](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237183) > The exponential backoff delay uses `base_delay * (2**attempt)`, where `attempt` starts at 0… The docstring has an off-by-one error. The code uses `base_delay * 2 ** attempt` where `attempt` is 0-indexed, giving delays of `base_delay * 1`, `base_delay * 2`, `base_delay * 4` for the 2nd, 3rd, 4th attempts — that is the intended behaviour. The docstring incorrectly states `base_delay * 2 ** (k - 2)` (which would give 0.5x on the second attempt); it should read `base_delay * 2 ** (k - 1)`. Will fix the docstring.
coding-agent-marvin8 commented 2026-06-11 02:43:52 +00:00 (Migrated from codeberg.org)

src/wuming/main.py line 44

When truncate_large_hunks replaces a hunk with a stub line, the line numbers of subsequent hunks may become incorrect…

No action needed — same reasoning as the diff.py comment above. hunk.new_start is parsed from the @@ ... +N,M @@ diff header and is independent of all other hunks. truncate_large_hunks only replaces hunk.lines; it never modifies new_start. build_position_map produces correct file-line keys for every hunk regardless of truncation.

[`src/wuming/main.py` line 44](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237186) > When truncate_large_hunks replaces a hunk with a stub line, the line numbers of subsequent hunks may become incorrect… No action needed — same reasoning as the `diff.py` comment above. `hunk.new_start` is parsed from the `@@ ... +N,M @@` diff header and is independent of all other hunks. `truncate_large_hunks` only replaces `hunk.lines`; it never modifies `new_start`. `build_position_map` produces correct file-line keys for every hunk regardless of truncation.
coding-agent-marvin8 commented 2026-06-11 02:44:00 +00:00 (Migrated from codeberg.org)

src/wuming/main.py line 87

The fallback logic for line number resolution (line 141-145) uses the original comment.line if quote-based lookup fails…

No action needed: if both the quote lookup and the fallback line number miss the diff, line_for_lookup not in position_maps.get(path, {}) is True and the comment is dropped with a warning. A bad agent line number therefore never produces a mis-anchored comment — it produces no comment. The quote mechanism improves resolution when it works; the validation gate handles the rest.

[`src/wuming/main.py` line 87](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237189) > The fallback logic for line number resolution (line 141-145) uses the original comment.line if quote-based lookup fails… No action needed: if both the quote lookup and the fallback line number miss the diff, `line_for_lookup not in position_maps.get(path, {})` is `True` and the comment is dropped with a warning. A bad agent line number therefore never produces a mis-anchored comment — it produces no comment. The quote mechanism improves resolution when it works; the validation gate handles the rest.
coding-agent-marvin8 commented 2026-06-11 02:44:07 +00:00 (Migrated from codeberg.org)

src/wuming/agents/base.py line 65

Using str(raw_quote) to convert the quote to a string will convert JSON null to the string 'None'…

No action needed: the guard if raw_quote is not None on the preceding line means str(raw_quote) is only called when raw_quote holds a real value. JSON null deserialises to Python None, which the guard catches, producing quote = None on the else branch.

[`src/wuming/agents/base.py` line 65](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237192) > Using `str(raw_quote)` to convert the quote to a string will convert JSON null to the string 'None'… No action needed: the guard `if raw_quote is not None` on the preceding line means `str(raw_quote)` is only called when `raw_quote` holds a real value. JSON `null` deserialises to Python `None`, which the guard catches, producing `quote = None` on the else branch.
coding-agent-marvin8 commented 2026-06-11 02:44:15 +00:00 (Migrated from codeberg.org)

src/wuming/backends/anthropic.py line 25

The inner function _attempt captures payload from the enclosing scope…

No action needed: payload, system, and user are constructed once before _attempt is defined and are never mutated between retry calls. system and user are immutable strings; payload is a dict built once and passed as a JSON body without modification. The closure is safe across all retry attempts.

[`src/wuming/backends/anthropic.py` line 25](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237195) > The inner function `_attempt` captures `payload` from the enclosing scope… No action needed: `payload`, `system`, and `user` are constructed once before `_attempt` is defined and are never mutated between retry calls. `system` and `user` are immutable strings; `payload` is a dict built once and passed as a JSON body without modification. The closure is safe across all retry attempts.
coding-agent-marvin8 commented 2026-06-11 02:44:22 +00:00 (Migrated from codeberg.org)

src/wuming/forgejo.py line 63

The inner function _fetch_inline captures rid via a default argument to bind the loop variable…

No action needed: default-argument binding is the standard Python idiom for capturing a loop variable in a closure. The type annotation rid: int makes the intent explicit. functools.partial would add an import for no readability gain in a 3-line helper.

[`src/wuming/forgejo.py` line 63](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237198) > The inner function `_fetch_inline` captures `rid` via a default argument to bind the loop variable… No action needed: default-argument binding is the standard Python idiom for capturing a loop variable in a closure. The type annotation `rid: int` makes the intent explicit. `functools.partial` would add an import for no readability gain in a 3-line helper.
coding-agent-marvin8 commented 2026-06-11 02:44:30 +00:00 (Migrated from codeberg.org)

src/wuming/forgejo.py line 63

The variable name rid shadows the built-in id function…

No action needed: rid does not shadow the built-in id. They are different names — Python's id is a standalone builtin; rid (review id) is a separate identifier with no relationship to it.

[`src/wuming/forgejo.py` line 63](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237201) > The variable name `rid` shadows the built-in `id` function… No action needed: `rid` does not shadow the built-in `id`. They are different names — Python's `id` is a standalone builtin; `rid` (review id) is a separate identifier with no relationship to it.
coding-agent-marvin8 commented 2026-06-11 02:44:37 +00:00 (Migrated from codeberg.org)

tests/test_log.py line 83

The regex pattern for ISO 8601 timestamp uses only digits and T, but does not account for fractional seconds…

No action needed: JSONFormatter.format() uses strftime("%Y-%m-%dT%H:%M:%S") which never emits fractional seconds. The regex in the test intentionally matches only what the formatter actually produces.

[`tests/test_log.py` line 83](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237204) > The regex pattern for ISO 8601 timestamp uses only digits and T, but does not account for fractional seconds… No action needed: `JSONFormatter.format()` uses `strftime("%Y-%m-%dT%H:%M:%S")` which never emits fractional seconds. The regex in the test intentionally matches only what the formatter actually produces.
coding-agent-marvin8 commented 2026-06-11 02:44:45 +00:00 (Migrated from codeberg.org)

CLAUDE.md line 5

The comment '# named config_agent to avoid shadowing wuming.config' refers to config_agent.py but is now adjacent to ret…

No action needed: the comment sits on the config_agent.py line of the project layout table, not on retry.py. retry.py appears two lines later with its own annotation. The layout is correct.

[`CLAUDE.md` line 5](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237207) > The comment '# named config_agent to avoid shadowing wuming.config' refers to config_agent.py but is now adjacent to ret… No action needed: the comment sits on the `config_agent.py` line of the project layout table, not on `retry.py`. `retry.py` appears two lines later with its own annotation. The layout is correct.
coding-agent-marvin8 commented 2026-06-11 02:44:52 +00:00 (Migrated from codeberg.org)

CLAUDE.md line 14

The description states 'post a plain comment', but it's unclear whether this is a PR comment or an inline comment…

Will fix: updating the wording to 'posts a plain PR comment and exits' in the WUMING_MAX_DIFF_LINES entry in CLAUDE.md, matching the README phrasing.

[`CLAUDE.md` line 14](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237210) > The description states 'post a plain comment', but it's unclear whether this is a PR comment or an inline comment… Will fix: updating the wording to 'posts a plain PR comment and exits' in the `WUMING_MAX_DIFF_LINES` entry in CLAUDE.md, matching the README phrasing.
coding-agent-marvin8 commented 2026-06-11 02:45:00 +00:00 (Migrated from codeberg.org)

README.md line 5

The default value in CLAUDE.md is described as '0 = disabled', but README.md uses a dash…

No action needed: README.md uses a dedicated Default column showing the raw value (0), with the meaning in the Description column. CLAUDE.md is an inline reference where 0 = disabled gives the meaning in one place without a separate column. Both are accurate for their context.

[`README.md` line 5](https://codeberg.org/marvin8/wuming/pulls/15#issuecomment-17237213) > The default value in CLAUDE.md is described as '0 = disabled', but README.md uses a dash… No action needed: README.md uses a dedicated Default column showing the raw value (`0`), with the meaning in the Description column. CLAUDE.md is an inline reference where `0 = disabled` gives the meaning in one place without a separate column. Both are accurate for their context.
Sign in to join this conversation.
No reviewers
No labels
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
marvin8/wuming!15
No description provided.