Add WUMING_MIN_SEVERITY severity floor #104
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No assignees
3 participants
Notifications
Due date
No due date set.
Reference
marvin8/wuming!104
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/issue-83-min-severity"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds a configurable minimum severity for posted findings (wave 1, PR 1).
WUMING_MIN_SEVERITY(low|medium|high|critical, unset = post everything) filters findings immediately after review, before position resolution and before the #82 verification pass — below-floor findings never cost LLM calls.ValueErrorat config load; unrecognised severity values fail closed when a floor is set.Refs #83
WuMing
Found 3 issue(s). See inline comments below.
@ -101,6 +101,7 @@ tools/- `WUMING_PROGRESS` — `true`/`1` force on; `false`/`0` force off; when unset or empty, auto-detects: enabled when stdout is a TTY and `CI` env var is absent- `WUMING_MAX_DIFF_LINES` — integer; if the raw diff line count exceeds this value and it is > 0, posts a plain PR comment and exits without inline review (default: `0` = disabled)- `WUMING_MAX_FILE_LINES` — integer; when > 0, fetches the full content of each changed file from Forgejo and includes it in the agent's context (files exceeding this line count are excluded); (default: `0` = disabled)- `WUMING_MIN_SEVERITY` — `low`|`medium`|`high`|`critical`; minimum severity for posted findings (below-floor and unrecognised severities are dropped); (default: unset = post everything)docs [MEDIUM]
Incomplete and ambiguous. The PR states that an invalid WUMING_MIN_SEVERITY value raises
ValueErrorat config load, but this entry never mentions that invalid config values are rejected — a reader could reasonably assume an unknown value silently falls back (contrast the neighbouringWUMING_VERIFYrow, which explicitly documents its fail-closed parsing). Additionally, the parenthetical "unrecognised severities are dropped" is stated unconditionally, yet per the implementation unknown finding severities only fail closed when a floor is actually set; when the variable is unset everything is posted. Fix: document that onlylow|medium|high|criticalare accepted for the env var (any other value is an error at startup), and scope the dropping sentence to findings evaluated against an active floor, e.g. "unrecognised finding severities are dropped when a floor is set".@ -158,6 +158,7 @@ See the [Forgejo/GitHub Actions guide](https://wuming.marvin8.zone/getting-start| `WUMING_PROGRESS` | — | `true`/`1` force on; `false`/`0` force off; when unset or empty, auto-detects: enabled when stdout is a TTY and `CI` env var is absent || `WUMING_MAX_DIFF_LINES` | `0` | Maximum raw diff size in lines. When the diff exceeds this value and it is greater than 0, WuMing posts a single plain PR comment explaining the PR is too large to review automatically, then exits without posting inline comments. Set to `0` to disable. || `WUMING_MAX_FILE_LINES` | `0` | Maximum file size (in lines) for full-file context fetching. When greater than 0, WuMing fetches the complete content of each changed file from Forgejo and includes it in the agent's context, giving the model visibility beyond the diff hunks. Files exceeding this line count are excluded. Set to `0` (the default) to disable. || `WUMING_MIN_SEVERITY` | *(unset)* | Minimum severity for posted findings: `low`, `medium`, `high`, or `critical`. Findings below the floor are dropped before posting (unrecognised severity values are dropped too). Unset by default — all findings are posted. |docs [MEDIUM]
Missing error behavior for the configuration value. The default column and prose cover the floor semantics, but unlike the
WUMING_VERIFYrow directly below ("Onlytrue/false... are accepted — any other value is an error, not a fallback to the default"), this row does not state that an unrecognisedWUMING_MIN_SEVERITYvalue raises an error at config load rather than being ignored. The sentence "unrecognised severity values are dropped too" also conflates the env var value with the severity of a finding, and its unconditional wording conflicts with "Unset by default — all findings are posted" (unknown finding severities are only dropped when a floor is set). Fix: state that onlylow/medium/high/criticalare accepted and that any other value is an error, and qualify the drop behavior as applying to findings when a floor is set.@ -96,6 +97,7 @@ async def _main() -> None:raw_comments = await router.review(file_diffs, backend, progress=progress, pr_info=pr_info)raw_comments = _apply_severity_floor(raw_comments, config.min_severity)code [MEDIUM]
The severity floor is applied at the correct point in the pipeline, but there is no integration-level test that verifies this ordering: that below-floor findings are dropped before _resolve_positions and before the verification LLM call. The PR's key efficiency claim is that dropped findings never cost verifier LLM calls, so add a test around _main/_review_and_publish that injects a below-floor finding and asserts the verifier backend and position resolver are not invoked for it.
src/wuming/main.py✅ Fixed in commit
b7ac816—test_pipeline_floor_before_verificationdrives one below-floor and one in-diff above-floor finding through_mainwithWUMING_MIN_SEVERITY=mediumand asserts the verifier receives only the above-floor finding and the review posts only it. Both findings sit on the same in-diff line, so position resolution would keep both — the floor is the only filter that can drop it.AGENTS.md✅ Fixed in commit
b7ac816— the entry now states that parsing is fail-closed: any other value raisesValueErrorat config load.README.md✅ Fixed in commit
b7ac816— the row now states only the four levels are accepted and any other value is an error at startup, not a fallback to posting everything.WuMing
Found 1 issue(s). See inline comments below.
@ -101,6 +101,7 @@ tools/- `WUMING_PROGRESS` — `true`/`1` force on; `false`/`0` force off; when unset or empty, auto-detects: enabled when stdout is a TTY and `CI` env var is absent- `WUMING_MAX_DIFF_LINES` — integer; if the raw diff line count exceeds this value and it is > 0, posts a plain PR comment and exits without inline review (default: `0` = disabled)- `WUMING_MAX_FILE_LINES` — integer; when > 0, fetches the full content of each changed file from Forgejo and includes it in the agent's context (files exceeding this line count are excluded); (default: `0` = disabled)- `WUMING_MIN_SEVERITY` — `low`|`medium`|`high`|`critical`; minimum severity for posted findings (below-floor and unrecognised severities are dropped); parsing is fail-closed — any other value raises `ValueError` at config load; (default: unset = post everything)docs [LOW]
Same ambiguity as the README table: the parenthetical states that unrecognised severities are dropped, while the default note says "unset = post everything". Unrecognised severity values only fail closed when a floor is configured, so the parenthetical should be qualified (e.g. "when a floor is set, below-floor and unrecognised severities are dropped") to match the actual behaviour and the PR description.
AGENTS.md✅ Fixed in commit pending-push — the parenthetical is now qualified: "when a floor is set, below-floor and unrecognised severities are dropped". The README table row carried the same unconditional phrasing, so it was qualified identically to keep the two documents consistent.
8a92bc0a18ca63fb8305