mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-10 22:43:37 +00:00
Refresh structured code review comments
This commit is contained in:
Bryan Helmkamp
parent
afa7298071
commit
1c35efb495
10 changed files with 619 additions and 88 deletions
|
|
@ -33,7 +33,7 @@ digraph CodeReview {
|
|||
timeout="300s",
|
||||
output_schema="routing",
|
||||
stdin_source="context.internal.run_id",
|
||||
script="python3 -c \"import hashlib,sys; pairs=list(zip(sys.argv[1::2],sys.argv[2::2])); sys.exit(0 if pairs and all(hashlib.sha256(open(path,'rb').read()).hexdigest()==expected for path,expected in pairs) else 91)\" .fabro/workflows/code-review/scripts/code_review.py e05d017c71a16c8418c1cf941ae134695520ccd6db62186f251706d7e635eafa .fabro/workflows/code-review/scripts/git_readonly.py bcd4364ba3aca2ee1e12d5909204f645c16bdf22e3753a39d74c79d8d37cf73e .fabro/workflows/code-review/scripts/publish_pr.py 2ba754164e91524f0b4b0dd6bb761d3ba64876ed21ed4f59a1b64ebd0ec52080 .fabro/workflows/code-review/scripts/render_report.py fd8f5eb237a75e3d1e946279b27dabf7d08757e2a6aac8ff1732b504ee3d2655 .fabro/workflows/code-review/scripts/rule_loader.py f885409c64c631075d7e31fcb6e7a100592430c5eba9a6e989222006061a9f52 .fabro/workflows/code-review/specs/report-spec.md 006898b9e84951237b3fe862bd07f9be77070fa8d9cc8d28ce528c5ccf270fda .fabro/workflows/code-review/templates/report.html 04e8bffbbec4c4848646fc5ff3261aa06dd3fa95b1dfa21474f7dec1b91abd22 .fabro/workflows/code-review/schemas/findings.schema.json 6fdf7c63fb6183c8bef64a874cd56f805d92c2aa3c011a513ee07f07b0797b6d .fabro/workflows/code-review/schemas/verdict.schema.json 4cb752e624f1b88809017ac5db472d59850ffe09ec5a93d3b63ba636364b8b19 .fabro/workflows/code-review/schemas/file-groups.schema.json b53c4e1c0bbd07bbf70e83f4f3b35fd96cb880c621c7c424e95b9aea34e13d7c .fabro/workflows/code-review/prompts/finder.md.j2 86c2e6a032f7c54c1bbab1c12496a8f0d6bf48703abe6017eb175330608cf223 .fabro/workflows/code-review/prompts/verify.md.j2 6528cffd1bf199f27a32c19547aebf6f0c78a546698aaef81c6a89441b80f7da .fabro/workflows/code-review/prompts/sweep.md.j2 e6f89b47b11c57030a6ef7d5896ccb37dbd2a2982e9fa7eb7f2e3df73acab82c .fabro/workflows/code-review/prompts/group-files.md.j2 5b291313a1266d1d658f80ea7989cdefcd8609b6893b7197b17914d553dab041 .fabro/workflows/code-review/prompts/partials/finding-fields.md.j2 b91ddeb86dc7f552323f7ac04823984da9365d013bfe7965c3eb6f51fa8abe0f .fabro/workflows/code-review/prompts/partials/guidance.md.j2 53bc0c40bb917288708bed1f9ba478fbd89b9790c92497762224cc752f40bef5 .fabro/workflows/code-review/prompts/partials/output-schema.md.j2 811994bb357739f2562d84f66dc05075ebe3c7f8d58034f8c25ee1c36bee996b .fabro/workflows/code-review/prompts/partials/read-only-explorer.md.j2 44a0244e7aa62fdb0dbbfdbadcffbfb640af249bae3e96895dedd5c7a33bad10 .fabro/workflows/code-review/prompts/partials/review-target.md.j2 abffeeff0e16b89a0754cd53f1833b3744494cd54ff761798b782a80467446ea .fabro/workflows/code-review/prompts/partials/safe-git-history.md.j2 4ddd8d36d5c51d7e166a6b7f1dff51b72cce0e64108cc7e892002ca909af8b3a .fabro/workflows/code-review/rules/builtin-manifest.json 47e3123fb25c560e36216dc9b8323c86e8301f1868a4fa3219bfd59bda3a533a && python3 .fabro/workflows/code-review/scripts/code_review.py prepare --review-id-stdin --mode {{ inputs.mode }} --effort {{ inputs.effort }} --scope {{ inputs.scope }} --base {{ inputs.base }} --commit {{ inputs.commit }} --range {{ inputs.range }} --model {{ inputs.model }} --guidance {{ inputs.guidance }}"
|
||||
script="python3 -c \"import hashlib,sys; pairs=list(zip(sys.argv[1::2],sys.argv[2::2])); sys.exit(0 if pairs and all(hashlib.sha256(open(path,'rb').read()).hexdigest()==expected for path,expected in pairs) else 91)\" .fabro/workflows/code-review/scripts/code_review.py 961fe81afa6a5c80432d8e8a05791d6c12ccc724e9f945e7190bbac77a6121d2 .fabro/workflows/code-review/scripts/git_readonly.py bcd4364ba3aca2ee1e12d5909204f645c16bdf22e3753a39d74c79d8d37cf73e .fabro/workflows/code-review/scripts/publish_pr.py fe65b5c710e6f486bf0b4a0da4933b53b954c32481313efd8d60d34b3dcc3a44 .fabro/workflows/code-review/scripts/render_report.py 91ea86428ed4759fbb368b4fa90ca8f54f1ab05f09f18bc8a4e9ffa188ed2295 .fabro/workflows/code-review/scripts/rule_loader.py f885409c64c631075d7e31fcb6e7a100592430c5eba9a6e989222006061a9f52 .fabro/workflows/code-review/specs/report-spec.md 7a54f72ee46f09218d18854d184a1f36875f9011877e94779c6b1f0d5dd118a9 .fabro/workflows/code-review/templates/report.html 5def570da34ca186da31781378367d70fb9c58e82f7aeec4aaf420fd348a8e61 .fabro/workflows/code-review/schemas/findings.schema.json 2f4d0a9052d5af0dad92db12a1e9d49cc91a282c4dddda495791352bf1559ed8 .fabro/workflows/code-review/schemas/verdict.schema.json de13ce02c5fd0c088640542831cc732e35dee3ddb38f89d4412f6a46fea75567 .fabro/workflows/code-review/schemas/file-groups.schema.json b53c4e1c0bbd07bbf70e83f4f3b35fd96cb880c621c7c424e95b9aea34e13d7c .fabro/workflows/code-review/prompts/finder.md.j2 86c2e6a032f7c54c1bbab1c12496a8f0d6bf48703abe6017eb175330608cf223 .fabro/workflows/code-review/prompts/verify.md.j2 cb3866240077d1bc8993b2f012a8d66a6ea61d4a9f2e88a1fefc6ef375f630e2 .fabro/workflows/code-review/prompts/sweep.md.j2 e6f89b47b11c57030a6ef7d5896ccb37dbd2a2982e9fa7eb7f2e3df73acab82c .fabro/workflows/code-review/prompts/group-files.md.j2 5b291313a1266d1d658f80ea7989cdefcd8609b6893b7197b17914d553dab041 .fabro/workflows/code-review/prompts/partials/finding-fields.md.j2 a81ee5b0ac134eb121dbf503025387c64126d3276e4673ebc836cfb62a3689fb .fabro/workflows/code-review/prompts/partials/guidance.md.j2 53bc0c40bb917288708bed1f9ba478fbd89b9790c92497762224cc752f40bef5 .fabro/workflows/code-review/prompts/partials/output-schema.md.j2 811994bb357739f2562d84f66dc05075ebe3c7f8d58034f8c25ee1c36bee996b .fabro/workflows/code-review/prompts/partials/read-only-explorer.md.j2 44a0244e7aa62fdb0dbbfdbadcffbfb640af249bae3e96895dedd5c7a33bad10 .fabro/workflows/code-review/prompts/partials/review-target.md.j2 abffeeff0e16b89a0754cd53f1833b3744494cd54ff761798b782a80467446ea .fabro/workflows/code-review/prompts/partials/safe-git-history.md.j2 4ddd8d36d5c51d7e166a6b7f1dff51b72cce0e64108cc7e892002ca909af8b3a .fabro/workflows/code-review/rules/builtin-manifest.json 47e3123fb25c560e36216dc9b8323c86e8301f1868a4fa3219bfd59bda3a533a && python3 .fabro/workflows/code-review/scripts/code_review.py prepare --review-id-stdin --mode {{ inputs.mode }} --effort {{ inputs.effort }} --scope {{ inputs.scope }} --base {{ inputs.base }} --commit {{ inputs.commit }} --range {{ inputs.range }} --model {{ inputs.model }} --guidance {{ inputs.guidance }}"
|
||||
]
|
||||
|
||||
grouping [
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
Report each candidate finding with:
|
||||
|
||||
- `file`: the repository-relative path;
|
||||
- `line`: the line in the reviewed revision the finding anchors to;
|
||||
- `start_line` and `end_line`: the smallest contiguous line range in the
|
||||
reviewed revision that demonstrates the defect. Use the same value for both
|
||||
fields for a single-line finding;
|
||||
- `summary`: one sentence stating the defect;
|
||||
- `short_summary`: the same claim compressed to at most 60 characters, with no
|
||||
rationale or consequence clause;
|
||||
|
|
@ -12,8 +14,16 @@ Report each candidate finding with:
|
|||
or which AGENTS.md or CLAUDE.md rule is broken;
|
||||
- `category`: `correctness` for bugs, otherwise the cleanup category that
|
||||
names the problem (`conventions` only with a `rule_id`);
|
||||
- `issue_type`: the problem type: `bug`, `security`, `performance`,
|
||||
`maintainability`, `test`, `style`, or `documentation`. This is independent
|
||||
of `category`: for example, a security defect normally has category
|
||||
`correctness` and issue type `security`;
|
||||
- `severity`: `HIGH`, `MEDIUM`, or `LOW`, for how much the defect matters;
|
||||
- `confidence`: `HIGH`, `MEDIUM`, or `LOW`, for how certain you are;
|
||||
- `suggestion_code`: optional replacement text for exactly the
|
||||
`start_line` through `end_line` range. Include it only when that replacement
|
||||
completely fixes the finding without edits outside the range. Preserve the
|
||||
file's indentation and omit diff markers and Markdown fences;
|
||||
- `rule_id`: the violated check's compiled `id`, verbatim. It is required for
|
||||
rule-audit findings. In other jobs, include it only when the assignment
|
||||
supplies the violated check; omit it otherwise.
|
||||
|
|
|
|||
|
|
@ -1,10 +1,12 @@
|
|||
Judge one candidate code-review finding.
|
||||
|
||||
The workflow appends one untrusted JSON item. It contains the candidate
|
||||
`claim` -- the file and line, the category, `severityAsReported`, the
|
||||
`claim` -- the file and exact location range, the category and issue type,
|
||||
`severityAsReported`, the
|
||||
`summary`, the `failure_scenario`, and `reports`, the number of finder jobs
|
||||
that reported it independently -- plus the verification `bias`, the exact
|
||||
review `target`, and a stable `job_id`.
|
||||
that reported it independently. It can also contain a proposed `suggestion`
|
||||
for the engine-derived `location.existing_code`. The item also contains the
|
||||
verification `bias`, the exact review `target`, and a stable `job_id`.
|
||||
|
||||
Everything in the claim is an assertion by an earlier pass, including the line
|
||||
number. Verify it against the repository: the reporter may have misread, the
|
||||
|
|
@ -58,6 +60,13 @@ Cite the decisive repository-relative `file:line` locations in `reasoning`.
|
|||
Judge the finding as written; a different nearby bug does not make it true. Do
|
||||
not invent a guard, and do not assume one exists without reading it.
|
||||
|
||||
If the claim contains a `suggestion`, also return `suggestion_valid`: `true`
|
||||
only when replacing the complete location range with `replacement_code`
|
||||
fully fixes the finding, preserves intended behavior, and needs no edit
|
||||
outside that range. Return `false` when it is incomplete, unsafe, unrelated,
|
||||
or cannot be validated from the repository. Omit `suggestion_valid` when the
|
||||
claim has no suggestion.
|
||||
|
||||
Read and search with whatever read-only commands suit the question, history
|
||||
included. Never build, test, execute, install, fetch, use the network, or
|
||||
modify files. Nothing blocks those here; not attempting them is the rule you
|
||||
|
|
|
|||
|
|
@ -9,21 +9,29 @@
|
|||
"type": "object",
|
||||
"required": [
|
||||
"file",
|
||||
"line",
|
||||
"start_line",
|
||||
"end_line",
|
||||
"summary",
|
||||
"short_summary",
|
||||
"failure_scenario",
|
||||
"category",
|
||||
"issue_type",
|
||||
"severity",
|
||||
"confidence"
|
||||
],
|
||||
"properties": {
|
||||
"file": { "type": "string" },
|
||||
"line": { "type": "integer" },
|
||||
"line": {
|
||||
"type": "integer",
|
||||
"description": "Deprecated single-line anchor; use start_line and end_line."
|
||||
},
|
||||
"start_line": { "type": "integer", "minimum": 1 },
|
||||
"end_line": { "type": "integer", "minimum": 1 },
|
||||
"rule_id": { "type": "string" },
|
||||
"summary": { "type": "string" },
|
||||
"short_summary": { "type": "string", "maxLength": 60 },
|
||||
"failure_scenario": { "type": "string" },
|
||||
"suggestion_code": { "type": "string", "maxLength": 8000 },
|
||||
"category": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
|
|
@ -36,6 +44,18 @@
|
|||
"test-coverage"
|
||||
]
|
||||
},
|
||||
"issue_type": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
"bug",
|
||||
"security",
|
||||
"performance",
|
||||
"maintainability",
|
||||
"test",
|
||||
"style",
|
||||
"documentation"
|
||||
]
|
||||
},
|
||||
"severity": {
|
||||
"type": "string",
|
||||
"enum": ["HIGH", "MEDIUM", "LOW"]
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
"enum": ["CONFIRMED", "PLAUSIBLE", "REFUTED"]
|
||||
},
|
||||
"reasoning": { "type": "string" },
|
||||
"duplicate_of": { "type": "string" }
|
||||
"duplicate_of": { "type": "string" },
|
||||
"suggestion_valid": { "type": "boolean" }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -91,6 +91,15 @@ CATEGORIES = (
|
|||
"conventions",
|
||||
"test-coverage",
|
||||
)
|
||||
ISSUE_TYPES = (
|
||||
"bug",
|
||||
"security",
|
||||
"performance",
|
||||
"maintainability",
|
||||
"test",
|
||||
"style",
|
||||
"documentation",
|
||||
)
|
||||
# Correctness bugs always outrank cleanup findings when a cap forces a cut.
|
||||
CLEANUP_CATEGORIES = frozenset(CATEGORIES) - {"correctness"}
|
||||
# Policy filters drop well-formed findings the review does not want; unlike a
|
||||
|
|
@ -116,6 +125,8 @@ SIBLING_CAP = 6
|
|||
CODE_FRAME_CONTEXT = 4
|
||||
CODE_FRAME_MAX_LINE_LENGTH = 400
|
||||
CODE_FRAME_MAX_BYTES = 2 * 1024 * 1024
|
||||
LOCATION_MAX_LINES = 50
|
||||
SUGGESTION_CODE_MAX_LENGTH = 8000
|
||||
CODE_FRAME_LANGUAGES = {
|
||||
"c": "C",
|
||||
"cc": "C++",
|
||||
|
|
@ -150,7 +161,7 @@ CODE_FRAME_LANGUAGES = {
|
|||
"yml": "YAML",
|
||||
}
|
||||
|
||||
CANONICAL_SCHEMA_VERSION = 3
|
||||
CANONICAL_SCHEMA_VERSION = 4
|
||||
CANONICAL_FILES = (
|
||||
"review-manifest.json",
|
||||
"candidate-ledger.jsonl",
|
||||
|
|
@ -1013,6 +1024,19 @@ def verify_schema_sources() -> None:
|
|||
f"{FINDINGS_SCHEMA_PATH} category enum does not match this "
|
||||
"engine's category list"
|
||||
)
|
||||
try:
|
||||
schema_issue_types = findings_schema["properties"]["findings"]["items"][
|
||||
"properties"
|
||||
]["issue_type"]["enum"]
|
||||
except (KeyError, TypeError) as error:
|
||||
raise WorkflowDataError(
|
||||
f"{FINDINGS_SCHEMA_PATH} has no issue_type enum"
|
||||
) from error
|
||||
if list(schema_issue_types) != list(ISSUE_TYPES):
|
||||
raise WorkflowDataError(
|
||||
f"{FINDINGS_SCHEMA_PATH} issue_type enum does not match this "
|
||||
"engine's issue type list"
|
||||
)
|
||||
verdict_schema = read_json(root() / VERDICT_SCHEMA_PATH)
|
||||
try:
|
||||
schema_verdicts = verdict_schema["properties"]["verdict"]["enum"]
|
||||
|
|
@ -1870,27 +1894,69 @@ def finding_or_rejection(
|
|||
if not isinstance(value, dict):
|
||||
return None, "the finding is not a JSON object"
|
||||
path = normalize_repo_path(value.get("file"))
|
||||
line = value.get("line")
|
||||
legacy_line = value.get("line")
|
||||
start_line = value.get("start_line", legacy_line)
|
||||
end_line = value.get("end_line", legacy_line)
|
||||
summary = one_line(value.get("summary"), 600).strip()
|
||||
short_summary = one_line(value.get("short_summary"), 200).strip()[:60]
|
||||
failure_scenario = clean_text(value.get("failure_scenario"), 4000).strip()
|
||||
category = one_line(value.get("category"), 40).strip().lower()
|
||||
issue_type = one_line(value.get("issue_type"), 40).strip().lower()
|
||||
severity = one_line(value.get("severity"), 20).upper()
|
||||
confidence = one_line(value.get("confidence"), 20).upper()
|
||||
raw_suggestion = value.get("suggestion_code", "")
|
||||
for failed, reason in (
|
||||
(path is None or path == ".", "file does not name a repository file"),
|
||||
(
|
||||
isinstance(line, bool) or not isinstance(line, int) or line < 1,
|
||||
"line is not a positive integer",
|
||||
isinstance(start_line, bool)
|
||||
or not isinstance(start_line, int)
|
||||
or start_line < 1,
|
||||
"start_line is not a positive integer",
|
||||
),
|
||||
(
|
||||
isinstance(end_line, bool)
|
||||
or not isinstance(end_line, int)
|
||||
or end_line < 1,
|
||||
"end_line is not a positive integer",
|
||||
),
|
||||
(
|
||||
isinstance(start_line, int)
|
||||
and isinstance(end_line, int)
|
||||
and start_line > end_line,
|
||||
"start_line is after end_line",
|
||||
),
|
||||
(
|
||||
isinstance(start_line, int)
|
||||
and isinstance(end_line, int)
|
||||
and end_line - start_line + 1 > LOCATION_MAX_LINES,
|
||||
f"location spans more than {LOCATION_MAX_LINES} lines",
|
||||
),
|
||||
(not summary, "summary is empty"),
|
||||
(not failure_scenario, "failure_scenario is empty"),
|
||||
(category not in CATEGORIES, "category is not in the closed list"),
|
||||
(issue_type not in ISSUE_TYPES, "issue_type is not in the closed list"),
|
||||
(severity not in SEVERITY_RANK, "severity is not HIGH, MEDIUM, or LOW"),
|
||||
(
|
||||
confidence not in CONFIDENCE_RANK,
|
||||
"confidence is not HIGH, MEDIUM, or LOW",
|
||||
),
|
||||
(
|
||||
not isinstance(raw_suggestion, str),
|
||||
"suggestion_code is not a string",
|
||||
),
|
||||
(
|
||||
isinstance(raw_suggestion, str)
|
||||
and len(raw_suggestion) > SUGGESTION_CODE_MAX_LENGTH,
|
||||
f"suggestion_code exceeds {SUGGESTION_CODE_MAX_LENGTH} characters",
|
||||
),
|
||||
(
|
||||
isinstance(raw_suggestion, str)
|
||||
and any(
|
||||
character not in "\n\t" and ord(character) < 0x20
|
||||
for character in raw_suggestion
|
||||
),
|
||||
"suggestion_code contains control characters",
|
||||
),
|
||||
):
|
||||
if failed:
|
||||
return None, reason
|
||||
|
|
@ -1922,13 +1988,20 @@ def finding_or_rejection(
|
|||
|
||||
return {
|
||||
"file": path,
|
||||
"line": line,
|
||||
# ``line`` remains the stable end-line alias used by ranking,
|
||||
# deduplication, and older consumers. The canonical finding also
|
||||
# carries the complete range.
|
||||
"line": end_line,
|
||||
"start_line": start_line,
|
||||
"end_line": end_line,
|
||||
"summary": summary,
|
||||
"short_summary": short_summary,
|
||||
"failure_scenario": failure_scenario,
|
||||
"category": category,
|
||||
"issue_type": issue_type,
|
||||
"severity": severity,
|
||||
"confidence": confidence,
|
||||
"suggestion_code": raw_suggestion if raw_suggestion.strip() else "",
|
||||
"rule_ids": rule_ids,
|
||||
}, None
|
||||
|
||||
|
|
@ -1975,7 +2048,7 @@ def state_rule_context(
|
|||
}
|
||||
|
||||
|
||||
def normalize_verdict(value: Any) -> Optional[Dict[str, str]]:
|
||||
def normalize_verdict(value: Any) -> Optional[Dict[str, Any]]:
|
||||
if not isinstance(value, dict):
|
||||
return None
|
||||
verdict = value.get("verdict")
|
||||
|
|
@ -1992,6 +2065,9 @@ def normalize_verdict(value: Any) -> Optional[Dict[str, str]]:
|
|||
duplicate_of.strip()
|
||||
):
|
||||
result["duplicate_of"] = duplicate_of.strip()
|
||||
suggestion_valid = value.get("suggestion_valid")
|
||||
if isinstance(suggestion_valid, bool):
|
||||
result["suggestion_valid"] = suggestion_valid
|
||||
return result
|
||||
|
||||
|
||||
|
|
@ -2338,16 +2414,26 @@ def verification_claim(
|
|||
about applicability, and the verifier judges only violation. ``pool``
|
||||
supplies the same-file siblings the verifier may name as duplicates.
|
||||
"""
|
||||
start_line = int(candidate.get("start_line") or candidate.get("line") or 0)
|
||||
end_line = int(candidate.get("end_line") or candidate.get("line") or 0)
|
||||
location = resolved_location(
|
||||
str(candidate.get("file") or ""), start_line, end_line
|
||||
)
|
||||
claim: Dict[str, Any] = {
|
||||
"file": candidate.get("file"),
|
||||
"line": candidate.get("line"),
|
||||
"location": location,
|
||||
"category": candidate.get("category"),
|
||||
"issue_type": candidate.get("issue_type"),
|
||||
"severityAsReported": candidate.get("severity"),
|
||||
"summary": candidate.get("summary"),
|
||||
"failure_scenario": candidate.get("failure_scenario"),
|
||||
"reports": int(candidate.get("reports") or 1),
|
||||
"siblings": sibling_claims(candidate, pool or []),
|
||||
}
|
||||
suggestion_code = str(candidate.get("suggestion_code") or "")
|
||||
if suggestion_code and location["existing_code"]:
|
||||
claim["suggestion"] = {"replacement_code": suggestion_code}
|
||||
rules_state = (state or {}).get("rules")
|
||||
if isinstance(rules_state, dict) and rules_state.get("enabled"):
|
||||
catalog = rules_state.get("catalog") or {}
|
||||
|
|
@ -2487,6 +2573,24 @@ def plan_verify() -> None:
|
|||
set(existing.get("rule_ids") or [])
|
||||
| set(report.get("rule_ids") or [])
|
||||
)
|
||||
# A fix is publishable only when reporting passes agree on its exact
|
||||
# range and replacement. A pass that offers no fix does not veto an
|
||||
# otherwise consistent proposal.
|
||||
existing_suggestion = str(existing.get("suggestion_code") or "")
|
||||
report_suggestion = str(report.get("suggestion_code") or "")
|
||||
if not existing_suggestion and report_suggestion:
|
||||
existing["start_line"] = report["start_line"]
|
||||
existing["end_line"] = report["end_line"]
|
||||
existing["line"] = report["end_line"]
|
||||
existing["suggestion_code"] = report_suggestion
|
||||
elif existing_suggestion and report_suggestion and (
|
||||
existing_suggestion != report_suggestion
|
||||
or existing.get("start_line") != report.get("start_line")
|
||||
or existing.get("end_line") != report.get("end_line")
|
||||
):
|
||||
existing["suggestion_code"] = ""
|
||||
if report.get("issue_type") == "security":
|
||||
existing["issue_type"] = "security"
|
||||
if (
|
||||
SEVERITY_RANK[report["severity"]]
|
||||
> SEVERITY_RANK[existing["severity"]]
|
||||
|
|
@ -2712,46 +2816,84 @@ def safe_code_text(value: str) -> str:
|
|||
return text
|
||||
|
||||
|
||||
def code_frame(file_path: str, line: int) -> Dict[str, Any]:
|
||||
"""Read the lines around a finding's anchor from the reviewed tree.
|
||||
def reviewed_source_lines(file_path: str) -> Optional[List[str]]:
|
||||
"""Read one UTF-8 source file from the unchanged reviewed tree."""
|
||||
target = root() / file_path
|
||||
try:
|
||||
if target.is_symlink() or not target.is_file():
|
||||
return None
|
||||
if target.stat().st_size > CODE_FRAME_MAX_BYTES:
|
||||
return None
|
||||
raw = target.read_bytes()
|
||||
except OSError:
|
||||
return None
|
||||
if b"\0" in raw:
|
||||
return None
|
||||
try:
|
||||
return raw.decode("utf-8").splitlines()
|
||||
except UnicodeError:
|
||||
return None
|
||||
|
||||
|
||||
def resolved_location(
|
||||
file_path: str, start_line: int, end_line: int
|
||||
) -> Dict[str, Any]:
|
||||
"""Build an engine-derived exact anchor for a finding."""
|
||||
existing_code = ""
|
||||
source_lines = reviewed_source_lines(file_path)
|
||||
if (
|
||||
source_lines is not None
|
||||
and 1 <= start_line <= end_line <= len(source_lines)
|
||||
):
|
||||
candidate = "\n".join(source_lines[start_line - 1:end_line])
|
||||
if (
|
||||
len(candidate) <= SUGGESTION_CODE_MAX_LENGTH
|
||||
and not any(
|
||||
character not in "\n\t" and ord(character) < 0x20
|
||||
for character in candidate
|
||||
)
|
||||
):
|
||||
existing_code = candidate
|
||||
return {
|
||||
"start_line": start_line,
|
||||
"end_line": end_line,
|
||||
"existing_code": existing_code,
|
||||
}
|
||||
|
||||
|
||||
def code_frame(
|
||||
file_path: str, start_line: int, end_line: Optional[int] = None
|
||||
) -> Dict[str, Any]:
|
||||
"""Read the lines around a finding's anchor range from the reviewed tree.
|
||||
|
||||
The excerpt shown in the report is read here, so its line numbers are the
|
||||
tree's own and no agent transcribes them. An unreadable, binary,
|
||||
oversized, or out-of-range target yields an empty excerpt.
|
||||
"""
|
||||
end_line = start_line if end_line is None else end_line
|
||||
language = code_frame_language(file_path)
|
||||
empty: Dict[str, Any] = {
|
||||
"language": language,
|
||||
"label": f"{file_path}:{line}",
|
||||
"label": f"{file_path}:{start_line}-{end_line}",
|
||||
"lines": [],
|
||||
}
|
||||
target = root() / file_path
|
||||
try:
|
||||
if target.is_symlink() or not target.is_file():
|
||||
return empty
|
||||
if target.stat().st_size > CODE_FRAME_MAX_BYTES:
|
||||
return empty
|
||||
raw = target.read_bytes()
|
||||
except OSError:
|
||||
source_lines = reviewed_source_lines(file_path)
|
||||
if (
|
||||
source_lines is None
|
||||
or start_line < 1
|
||||
or end_line < start_line
|
||||
or end_line > len(source_lines)
|
||||
):
|
||||
return empty
|
||||
if b"\0" in raw:
|
||||
return empty
|
||||
try:
|
||||
text = raw.decode("utf-8")
|
||||
except UnicodeError:
|
||||
return empty
|
||||
source_lines = text.splitlines()
|
||||
if line > len(source_lines):
|
||||
return empty
|
||||
start = max(1, line - CODE_FRAME_CONTEXT)
|
||||
end = min(len(source_lines), line + CODE_FRAME_CONTEXT)
|
||||
start = max(1, start_line - CODE_FRAME_CONTEXT)
|
||||
end = min(len(source_lines), end_line + CODE_FRAME_CONTEXT)
|
||||
lines: List[Dict[str, Any]] = []
|
||||
for number in range(start, end + 1):
|
||||
entry: Dict[str, Any] = {
|
||||
"number": number,
|
||||
"text": safe_code_text(source_lines[number - 1]),
|
||||
}
|
||||
if number == line:
|
||||
if start_line <= number <= end_line:
|
||||
entry["highlight"] = True
|
||||
lines.append(entry)
|
||||
return {
|
||||
|
|
@ -2767,14 +2909,19 @@ def reportable_finding(
|
|||
) -> Dict[str, Any]:
|
||||
candidate = record["candidate"]
|
||||
verdict = record.get("verdict")
|
||||
return {
|
||||
start_line = int(candidate.get("start_line") or candidate["line"])
|
||||
end_line = int(candidate.get("end_line") or candidate["line"])
|
||||
location = resolved_location(candidate["file"], start_line, end_line)
|
||||
finding = {
|
||||
"id": display_id,
|
||||
"file": candidate["file"],
|
||||
"line": candidate["line"],
|
||||
"line": end_line,
|
||||
"location": location,
|
||||
"summary": candidate["summary"],
|
||||
"short_summary": candidate["short_summary"],
|
||||
"failure_scenario": candidate["failure_scenario"],
|
||||
"category": candidate["category"],
|
||||
"issue_type": candidate["issue_type"],
|
||||
"severity": candidate["severity"],
|
||||
"confidence": candidate["confidence"],
|
||||
"reports": int(candidate.get("reports") or 1),
|
||||
|
|
@ -2785,8 +2932,18 @@ def reportable_finding(
|
|||
"source": candidate.get("source", "finder"),
|
||||
"verdict": verdict["verdict"] if verdict else "UNVERIFIED",
|
||||
"verdict_reasoning": verdict["reasoning"] if verdict else "",
|
||||
"code": code_frame(candidate["file"], int(candidate["line"])),
|
||||
"code": code_frame(candidate["file"], start_line, end_line),
|
||||
}
|
||||
suggestion_code = str(candidate.get("suggestion_code") or "")
|
||||
if (
|
||||
suggestion_code
|
||||
and location["existing_code"]
|
||||
and verdict is not None
|
||||
and verdict.get("suggestion_valid") is True
|
||||
and suggestion_code != location["existing_code"]
|
||||
):
|
||||
finding["suggestion"] = {"replacement_code": suggestion_code}
|
||||
return finding
|
||||
|
||||
|
||||
def finding_reports(state: Mapping[str, Any], key: str) -> List[str]:
|
||||
|
|
@ -2825,6 +2982,8 @@ def vote_records(
|
|||
entry["reasoning"] = verdict["reasoning"]
|
||||
if verdict.get("duplicate_of"):
|
||||
entry["duplicate_of"] = verdict["duplicate_of"]
|
||||
if "suggestion_valid" in verdict:
|
||||
entry["suggestion_valid"] = verdict["suggestion_valid"]
|
||||
records.append(entry)
|
||||
return records
|
||||
|
||||
|
|
@ -3103,7 +3262,10 @@ def final_tally() -> None:
|
|||
"id": candidate.get("id"),
|
||||
"file": candidate.get("file"),
|
||||
"line": candidate.get("line"),
|
||||
"start_line": candidate.get("start_line"),
|
||||
"end_line": candidate.get("end_line"),
|
||||
"category": candidate.get("category"),
|
||||
"issue_type": candidate.get("issue_type"),
|
||||
"severity": candidate.get("severity"),
|
||||
"confidence": candidate.get("confidence"),
|
||||
"reports": int(candidate.get("reports") or 1),
|
||||
|
|
@ -3114,6 +3276,8 @@ def final_tally() -> None:
|
|||
"failure_scenario": candidate.get("failure_scenario"),
|
||||
"disposition": disposition,
|
||||
}
|
||||
if candidate.get("suggestion_code"):
|
||||
entry["suggestion_code"] = candidate["suggestion_code"]
|
||||
if verdict is not None:
|
||||
entry["verdict"] = verdict["verdict"]
|
||||
if disposition == "duplicate":
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ DEFAULT_BATCH_SIZE = 50
|
|||
SUMMARY_BUDGET = 65000
|
||||
GITHUB_BODY_CAP = 65536
|
||||
SEVERITY_RANK = {"LOW": 0, "MEDIUM": 1, "HIGH": 2}
|
||||
SEVERITY_EMOJI = {"LOW": "🟡", "MEDIUM": "🟠", "HIGH": "🔴"}
|
||||
CANONICAL_FILE_NAMES = (
|
||||
"review-manifest.json",
|
||||
"candidate-ledger.jsonl",
|
||||
|
|
@ -68,6 +69,14 @@ UNVERIFIED_NOTE = (
|
|||
"_This finding comes from a low-effort single-pass review and was not "
|
||||
"independently verified._"
|
||||
)
|
||||
PLAUSIBLE_WARNING = (
|
||||
"> **Needs confirmation:** The verifier could not fully confirm this "
|
||||
"finding from the available evidence."
|
||||
)
|
||||
UNVERIFIED_WARNING = (
|
||||
"> **Not verified:** This finding comes from a low-effort single-pass "
|
||||
"review and was not independently verified."
|
||||
)
|
||||
|
||||
|
||||
class PublishError(RuntimeError):
|
||||
|
|
@ -201,6 +210,19 @@ def has_diff_position(
|
|||
return any(start <= line <= end for start, end in hunks.get(path, ()))
|
||||
|
||||
|
||||
def has_diff_range(
|
||||
hunks: Mapping[str, Sequence[Tuple[int, int]]],
|
||||
path: str,
|
||||
start_line: int,
|
||||
end_line: int,
|
||||
) -> bool:
|
||||
"""True when one RIGHT-side hunk contains the complete range."""
|
||||
return any(
|
||||
hunk_start <= start_line <= end_line <= hunk_end
|
||||
for hunk_start, hunk_end in hunks.get(path, ())
|
||||
)
|
||||
|
||||
|
||||
# --- Routing configuration (R3-R5, fail-closed) ------------------------------
|
||||
|
||||
|
||||
|
|
@ -296,6 +318,11 @@ def safe_code_block(code: Mapping[str, Any]) -> List[str]:
|
|||
return body
|
||||
|
||||
|
||||
def raw_code_block(text: str, language: str = "text") -> List[str]:
|
||||
fence = backtick_fence([text])
|
||||
return [fence + language, text, fence]
|
||||
|
||||
|
||||
def finding_detail_lines(finding: Mapping[str, Any]) -> List[str]:
|
||||
lines: List[str] = []
|
||||
if finding["summary"].strip() != finding["short_summary"].strip():
|
||||
|
|
@ -318,29 +345,99 @@ def finding_detail_lines(finding: Mapping[str, Any]) -> List[str]:
|
|||
return lines
|
||||
|
||||
|
||||
def inline_more_lines(finding: Mapping[str, Any]) -> List[str]:
|
||||
verdict = str(finding["verdict"]).lower().capitalize()
|
||||
confidence = str(finding["confidence"]).lower().capitalize()
|
||||
lines = [
|
||||
"",
|
||||
"<details>",
|
||||
f"<summary>More · {verdict} · {confidence} confidence</summary>",
|
||||
"",
|
||||
"**Impact:** "
|
||||
+ renderer.escape_markdown(finding["failure_scenario"]),
|
||||
"",
|
||||
]
|
||||
reasoning = str(finding.get("verdict_reasoning") or "").strip()
|
||||
if reasoning:
|
||||
evidence = renderer.escape_markdown(reasoning)
|
||||
elif finding["verdict"] == "UNVERIFIED":
|
||||
evidence = "No independent verification ran at this effort level."
|
||||
else:
|
||||
evidence = "No verifier reasoning was recorded."
|
||||
lines.append("**Evidence:** " + evidence)
|
||||
|
||||
metadata: List[str] = []
|
||||
reports = finding.get("reports")
|
||||
reporters = finding.get("reporters") or []
|
||||
if isinstance(reports, int) and reports > 1:
|
||||
report_text = f"- Reported by {reports} review passes"
|
||||
if reporters:
|
||||
report_text += ": " + renderer.escape_markdown(
|
||||
", ".join(str(reporter) for reporter in reporters)
|
||||
)
|
||||
metadata.append(report_text)
|
||||
|
||||
rule_ids = finding.get("rule_ids") or []
|
||||
if rule_ids:
|
||||
label = "Rule" if len(rule_ids) == 1 else "Rules"
|
||||
metadata.append(
|
||||
f"- {label}: "
|
||||
+ ", ".join(renderer.code_span(rule_id) for rule_id in rule_ids)
|
||||
)
|
||||
|
||||
for anchor in finding.get("anchors") or []:
|
||||
location = f"{anchor['file']}:{anchor['line']}"
|
||||
metadata.append(
|
||||
f"- Related location: {renderer.code_span(location)} "
|
||||
f"({renderer.escape_markdown(anchor['category'])}, "
|
||||
f"{renderer.escape_markdown(anchor['id'])})"
|
||||
)
|
||||
|
||||
if metadata:
|
||||
lines.extend(["", *metadata])
|
||||
lines.extend(["", "</details>"])
|
||||
return lines
|
||||
|
||||
|
||||
def inline_comment_body(finding: Mapping[str, Any], review_id: str) -> str:
|
||||
tag = comment_tag(review_id, finding["id"])
|
||||
issue_type = str(finding["issue_type"]).lower().capitalize()
|
||||
lines = [
|
||||
f"<!-- {tag} -->",
|
||||
"",
|
||||
f"**{finding['severity']} · {finding['category']}** — "
|
||||
f"**{SEVERITY_EMOJI[finding['severity']]} {issue_type}** — "
|
||||
+ renderer.escape_markdown(finding["short_summary"]),
|
||||
*finding_detail_lines(finding),
|
||||
]
|
||||
meta = f"Verdict {finding['verdict']} · confidence {finding['confidence']}"
|
||||
rule_ids = finding.get("rule_ids") or []
|
||||
if rule_ids:
|
||||
meta += " · rule " + ", ".join(
|
||||
renderer.code_span(rule_id) for rule_id in rule_ids
|
||||
if finding["summary"].strip() != finding["short_summary"].strip():
|
||||
lines.extend(["", renderer.escape_markdown(finding["summary"])])
|
||||
if finding["verdict"] == "PLAUSIBLE":
|
||||
lines.extend(["", PLAUSIBLE_WARNING])
|
||||
elif finding["verdict"] == "UNVERIFIED":
|
||||
lines.extend(["", UNVERIFIED_WARNING])
|
||||
suggestion = finding.get("suggestion")
|
||||
if isinstance(suggestion, dict):
|
||||
lines.extend(
|
||||
[
|
||||
"",
|
||||
*raw_code_block(suggestion["replacement_code"], "suggestion"),
|
||||
]
|
||||
)
|
||||
lines.extend(["", f"_{meta}_"])
|
||||
lines.extend(inline_more_lines(finding))
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def summary_section(
|
||||
finding: Mapping[str, Any], reason_text: Optional[str]
|
||||
) -> str:
|
||||
location = renderer.code_span(f"{finding['file']}:{finding['line']}")
|
||||
location_data = finding["location"]
|
||||
start_line = location_data["start_line"]
|
||||
end_line = location_data["end_line"]
|
||||
location_text = (
|
||||
f"{finding['file']}:{start_line}"
|
||||
if start_line == end_line
|
||||
else f"{finding['file']}:{start_line}-{end_line}"
|
||||
)
|
||||
location = renderer.code_span(location_text)
|
||||
facts = [location]
|
||||
if reason_text:
|
||||
facts.append(reason_text)
|
||||
|
|
@ -353,7 +450,8 @@ def summary_section(
|
|||
+ ", ".join(renderer.code_span(rule_id) for rule_id in rule_ids)
|
||||
)
|
||||
lines = [
|
||||
f"### {finding['id']} · {finding['severity']} {finding['category']} — "
|
||||
f"### {finding['id']} · {finding['severity']} "
|
||||
f"{finding['issue_type']} / {finding['category']} — "
|
||||
+ renderer.escape_markdown(finding["short_summary"]),
|
||||
"",
|
||||
" · ".join(facts),
|
||||
|
|
@ -362,6 +460,22 @@ def summary_section(
|
|||
excerpt = safe_code_block(finding["code"])
|
||||
if excerpt:
|
||||
lines.extend(["", *excerpt])
|
||||
suggestion = finding.get("suggestion")
|
||||
if isinstance(suggestion, dict):
|
||||
lines.extend(
|
||||
[
|
||||
"",
|
||||
"<details><summary>Suggested change</summary>",
|
||||
"",
|
||||
"**Before:**",
|
||||
*raw_code_block(location_data["existing_code"]),
|
||||
"",
|
||||
"**After:**",
|
||||
*raw_code_block(suggestion["replacement_code"]),
|
||||
"",
|
||||
"</details>",
|
||||
]
|
||||
)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
|
|
@ -597,12 +711,19 @@ def command_plan(args: argparse.Namespace) -> int:
|
|||
# no-position reason and routing can never hide a placement.
|
||||
placements: List[Dict[str, Any]] = []
|
||||
for finding in findings:
|
||||
location = finding["location"]
|
||||
start_line = location["start_line"]
|
||||
end_line = location["end_line"]
|
||||
base_entry = {
|
||||
"finding_id": finding["id"],
|
||||
"path": finding["file"],
|
||||
"line": finding["line"],
|
||||
"line": end_line,
|
||||
"start_line": start_line,
|
||||
"end_line": end_line,
|
||||
}
|
||||
if not has_diff_position(hunks, finding["file"], finding["line"]):
|
||||
if not has_diff_range(
|
||||
hunks, finding["file"], start_line, end_line
|
||||
):
|
||||
placements.append(
|
||||
{
|
||||
**base_entry,
|
||||
|
|
@ -811,6 +932,24 @@ def validate_plan_document(
|
|||
line = entry.get("line")
|
||||
if isinstance(line, bool) or not isinstance(line, int) or line < 1:
|
||||
fail(f"plan {field}.line must be a positive integer")
|
||||
start_line = entry.get("start_line")
|
||||
end_line = entry.get("end_line")
|
||||
if (
|
||||
isinstance(start_line, bool)
|
||||
or not isinstance(start_line, int)
|
||||
or start_line < 1
|
||||
):
|
||||
fail(f"plan {field}.start_line must be a positive integer")
|
||||
if (
|
||||
isinstance(end_line, bool)
|
||||
or not isinstance(end_line, int)
|
||||
or end_line < start_line
|
||||
or end_line != line
|
||||
):
|
||||
fail(
|
||||
f"plan {field}.end_line must end at its line and not precede "
|
||||
"start_line"
|
||||
)
|
||||
body = require_text(entry.get("body"), f"{field}.body", GITHUB_BODY_CAP)
|
||||
placement = entry.get("placement")
|
||||
if placement == "inline":
|
||||
|
|
@ -1014,15 +1153,19 @@ class BatchPoster:
|
|||
self.batches_succeeded = 0
|
||||
|
||||
def post_review(self, finding_ids: Sequence[str]) -> Optional[int]:
|
||||
comments = [
|
||||
{
|
||||
"path": self.by_id[finding_id]["path"],
|
||||
"line": self.by_id[finding_id]["line"],
|
||||
comments: List[Dict[str, Any]] = []
|
||||
for finding_id in finding_ids:
|
||||
entry = self.by_id[finding_id]
|
||||
comment: Dict[str, Any] = {
|
||||
"path": entry["path"],
|
||||
"line": entry["end_line"],
|
||||
"side": "RIGHT",
|
||||
"body": self.by_id[finding_id]["body"],
|
||||
"body": entry["body"],
|
||||
}
|
||||
for finding_id in finding_ids
|
||||
]
|
||||
if entry["start_line"] != entry["end_line"]:
|
||||
comment["start_line"] = entry["start_line"]
|
||||
comment["start_side"] = "RIGHT"
|
||||
comments.append(comment)
|
||||
# No review body: the run tag lives in the sticky summary and in
|
||||
# every inline comment's identity tag, so body text here would
|
||||
# only add a noise bubble to the PR timeline (R12).
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ Python 3.9-compatible. Standard library only.
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import hashlib
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
|
@ -22,7 +23,7 @@ from typing import Any, Dict, List, Mapping, NoReturn, Sequence, Tuple
|
|||
from urllib.parse import quote
|
||||
|
||||
|
||||
CANONICAL_SCHEMA_VERSION = 3
|
||||
CANONICAL_SCHEMA_VERSION = 4
|
||||
TEMPLATE_RELATIVE_PATH = ("..", "templates", "report.html")
|
||||
PAYLOAD_PLACEHOLDER = "__CODE_REVIEW_PAYLOAD__"
|
||||
|
||||
|
|
@ -35,6 +36,15 @@ CATEGORIES = (
|
|||
"conventions",
|
||||
"test-coverage",
|
||||
)
|
||||
ISSUE_TYPES = (
|
||||
"bug",
|
||||
"security",
|
||||
"performance",
|
||||
"maintainability",
|
||||
"test",
|
||||
"style",
|
||||
"documentation",
|
||||
)
|
||||
SEVERITIES = ("HIGH", "MEDIUM", "LOW")
|
||||
FINDING_VERDICTS = ("CONFIRMED", "PLAUSIBLE", "UNVERIFIED")
|
||||
VOTE_VERDICTS = ("CONFIRMED", "PLAUSIBLE", "REFUTED")
|
||||
|
|
@ -58,6 +68,7 @@ COMPILED_RULE_ID_RE = re.compile(
|
|||
)
|
||||
MAX_TEXT = 8000
|
||||
MAX_RULE_IDS_PER_FINDING = 50
|
||||
MAX_LOCATION_LINES = 50
|
||||
|
||||
|
||||
class RenderError(RuntimeError):
|
||||
|
|
@ -211,8 +222,8 @@ def validate_code(value: object, field: str) -> Dict[str, Any]:
|
|||
line["highlight"] = True
|
||||
highlighted += 1
|
||||
normalized.append(line)
|
||||
if normalized and highlighted != 1:
|
||||
die(f"{field} must highlight exactly one line")
|
||||
if normalized and highlighted < 1:
|
||||
die(f"{field} must highlight at least one line")
|
||||
return {
|
||||
"language": code["language"],
|
||||
"label": code["label"],
|
||||
|
|
@ -230,12 +241,49 @@ def validate_finding(value: object, index: int) -> Dict[str, Any]:
|
|||
line = positive_int(finding.get("line"), f"{field}.line")
|
||||
if finding.get("category") not in CATEGORIES:
|
||||
die(f"{field}.category is not in the closed list")
|
||||
if finding.get("issue_type") not in ISSUE_TYPES:
|
||||
die(f"{field}.issue_type is not in the closed list")
|
||||
if finding.get("severity") not in SEVERITIES:
|
||||
die(f"{field}.severity is invalid")
|
||||
if finding.get("confidence") not in SEVERITIES:
|
||||
die(f"{field}.confidence is invalid")
|
||||
if finding.get("verdict") not in FINDING_VERDICTS:
|
||||
die(f"{field}.verdict is invalid")
|
||||
location = as_map(finding.get("location"))
|
||||
start_line = positive_int(
|
||||
location.get("start_line"), f"{field}.location.start_line"
|
||||
)
|
||||
end_line = positive_int(
|
||||
location.get("end_line"), f"{field}.location.end_line"
|
||||
)
|
||||
if start_line > end_line:
|
||||
die(f"{field}.location starts after it ends")
|
||||
if end_line - start_line + 1 > MAX_LOCATION_LINES:
|
||||
die(f"{field}.location spans more than {MAX_LOCATION_LINES} lines")
|
||||
if end_line != line:
|
||||
die(f"{field}.line must equal location.end_line")
|
||||
normalized_location = {
|
||||
"start_line": start_line,
|
||||
"end_line": end_line,
|
||||
"existing_code": safe_text(
|
||||
location.get("existing_code"),
|
||||
f"{field}.location.existing_code",
|
||||
),
|
||||
}
|
||||
suggestion = finding.get("suggestion")
|
||||
normalized_suggestion: Optional[Dict[str, str]] = None
|
||||
if suggestion is not None:
|
||||
suggestion_record = as_map(suggestion)
|
||||
replacement = safe_text(
|
||||
suggestion_record.get("replacement_code"),
|
||||
f"{field}.suggestion.replacement_code",
|
||||
allow_empty=False,
|
||||
)
|
||||
if not normalized_location["existing_code"]:
|
||||
die(f"{field}.suggestion has no exact existing code anchor")
|
||||
if replacement == normalized_location["existing_code"]:
|
||||
die(f"{field}.suggestion does not change the anchored code")
|
||||
normalized_suggestion = {"replacement_code": replacement}
|
||||
reporters = finding.get("reporters")
|
||||
if not isinstance(reporters, list) or not all(
|
||||
isinstance(item, str) for item in reporters
|
||||
|
|
@ -270,10 +318,21 @@ def validate_finding(value: object, index: int) -> Dict[str, Any]:
|
|||
"category": record["category"],
|
||||
}
|
||||
)
|
||||
return {
|
||||
normalized_code = validate_code(finding.get("code"), f"{field}.code")
|
||||
if normalized_code["lines"]:
|
||||
highlighted = {
|
||||
entry["number"]
|
||||
for entry in normalized_code["lines"]
|
||||
if entry.get("highlight")
|
||||
}
|
||||
expected = set(range(start_line, end_line + 1))
|
||||
if highlighted != expected:
|
||||
die(f"{field}.code highlights do not match its location")
|
||||
normalized = {
|
||||
"id": display_id,
|
||||
"file": path,
|
||||
"line": line,
|
||||
"location": normalized_location,
|
||||
"summary": safe_text(
|
||||
finding.get("summary"), f"{field}.summary", allow_empty=False
|
||||
),
|
||||
|
|
@ -288,6 +347,7 @@ def validate_finding(value: object, index: int) -> Dict[str, Any]:
|
|||
allow_empty=False,
|
||||
),
|
||||
"category": finding["category"],
|
||||
"issue_type": finding["issue_type"],
|
||||
"severity": finding["severity"],
|
||||
"confidence": finding["confidence"],
|
||||
"reports": positive_int(finding.get("reports"), f"{field}.reports"),
|
||||
|
|
@ -299,8 +359,11 @@ def validate_finding(value: object, index: int) -> Dict[str, Any]:
|
|||
"verdict_reasoning": safe_text(
|
||||
finding.get("verdict_reasoning"), f"{field}.verdict_reasoning"
|
||||
),
|
||||
"code": validate_code(finding.get("code"), f"{field}.code"),
|
||||
"code": normalized_code,
|
||||
}
|
||||
if normalized_suggestion is not None:
|
||||
normalized["suggestion"] = normalized_suggestion
|
||||
return normalized
|
||||
|
||||
|
||||
def validate_findings(value: object) -> List[Dict[str, Any]]:
|
||||
|
|
@ -333,6 +396,8 @@ def validate_ledger(records: Sequence[Mapping[str, Any]]) -> List[Dict[str, Any]
|
|||
positive_int(record.get("line"), f"{field}.line")
|
||||
if record.get("category") not in CATEGORIES:
|
||||
die(f"{field}.category is not in the closed list")
|
||||
if record.get("issue_type") not in ISSUE_TYPES:
|
||||
die(f"{field}.issue_type is not in the closed list")
|
||||
if record.get("disposition") not in DISPOSITIONS:
|
||||
die(f"{field}.disposition is invalid")
|
||||
validated.append(dict(record))
|
||||
|
|
@ -351,6 +416,10 @@ def validate_votes(records: Sequence[Mapping[str, Any]]) -> List[Dict[str, Any]]
|
|||
if record.get("verdict") not in VOTE_VERDICTS:
|
||||
die(f"{field}.verdict is invalid")
|
||||
safe_text(record.get("reasoning"), f"{field}.reasoning")
|
||||
if "suggestion_valid" in record and not isinstance(
|
||||
record.get("suggestion_valid"), bool
|
||||
):
|
||||
die(f"{field}.suggestion_valid must be a boolean")
|
||||
validated.append(dict(record))
|
||||
return validated
|
||||
|
||||
|
|
@ -582,6 +651,12 @@ def code_block(code: Mapping[str, Any]) -> List[str]:
|
|||
return body
|
||||
|
||||
|
||||
def fenced_text(value: str, language: str = "text") -> List[str]:
|
||||
longest = max((len(run) for run in re.findall(r"`+", value)), default=0)
|
||||
fence = "`" * max(4, longest + 1)
|
||||
return [fence + language, value, fence]
|
||||
|
||||
|
||||
def describe_target(manifest: Mapping[str, Any]) -> str:
|
||||
mode = str(manifest.get("mode"))
|
||||
if mode == "files":
|
||||
|
|
@ -601,11 +676,19 @@ def revision_summary(manifest: Mapping[str, Any]) -> str:
|
|||
|
||||
|
||||
def finding_markdown(finding: Mapping[str, Any]) -> List[str]:
|
||||
location = f"{finding['file']}:{finding['line']}"
|
||||
location_data = finding["location"]
|
||||
start_line = location_data["start_line"]
|
||||
end_line = location_data["end_line"]
|
||||
location = (
|
||||
f"{finding['file']}:{start_line}"
|
||||
if start_line == end_line
|
||||
else f"{finding['file']}:{start_line}-{end_line}"
|
||||
)
|
||||
rule_ids = finding.get("rule_ids") or []
|
||||
lines = [
|
||||
f"### {finding['id']} · {finding['severity']} "
|
||||
f"{finding['category']} — {escape_markdown(finding['short_summary'])}",
|
||||
f"{finding['issue_type']} / {finding['category']} — "
|
||||
f"{escape_markdown(finding['short_summary'])}",
|
||||
"",
|
||||
f"{code_span(location)} · verdict {finding['verdict']} · "
|
||||
f"confidence {finding['confidence']} · reported by "
|
||||
|
|
@ -636,6 +719,22 @@ def finding_markdown(finding: Mapping[str, Any]) -> List[str]:
|
|||
reasoning = str(finding.get("verdict_reasoning") or "").strip()
|
||||
if reasoning:
|
||||
lines.extend(["", f"**Verifier.** {escape_markdown(reasoning)}"])
|
||||
suggestion = finding.get("suggestion")
|
||||
if isinstance(suggestion, dict):
|
||||
lines.extend(
|
||||
[
|
||||
"",
|
||||
"<details><summary>Suggested change</summary>",
|
||||
"",
|
||||
"**Before:**",
|
||||
*fenced_text(location_data["existing_code"]),
|
||||
"",
|
||||
"**After:**",
|
||||
*fenced_text(suggestion["replacement_code"]),
|
||||
"",
|
||||
"</details>",
|
||||
]
|
||||
)
|
||||
excerpt = code_block(finding["code"])
|
||||
if excerpt:
|
||||
lines.extend(["", *excerpt])
|
||||
|
|
@ -841,7 +940,9 @@ def jsonl_line(finding: Mapping[str, Any]) -> str:
|
|||
"id",
|
||||
"file",
|
||||
"line",
|
||||
"location",
|
||||
"category",
|
||||
"issue_type",
|
||||
"severity",
|
||||
"confidence",
|
||||
"verdict",
|
||||
|
|
@ -854,6 +955,8 @@ def jsonl_line(finding: Mapping[str, Any]) -> str:
|
|||
"source",
|
||||
)
|
||||
}
|
||||
if finding.get("suggestion") is not None:
|
||||
record["suggestion"] = finding["suggestion"]
|
||||
return json.dumps(record, ensure_ascii=False, separators=(",", ":"))
|
||||
|
||||
|
||||
|
|
@ -889,14 +992,19 @@ def sarif_uri(path: str) -> str:
|
|||
return quote(path, safe="/")
|
||||
|
||||
|
||||
def sarif_location(path: str, line: int) -> Dict[str, Any]:
|
||||
def sarif_location(
|
||||
path: str, start_line: int, end_line: Optional[int] = None
|
||||
) -> Dict[str, Any]:
|
||||
region: Dict[str, Any] = {"startLine": start_line}
|
||||
if end_line is not None and end_line != start_line:
|
||||
region["endLine"] = end_line
|
||||
return {
|
||||
"physicalLocation": {
|
||||
"artifactLocation": {
|
||||
"uri": sarif_uri(path),
|
||||
"uriBaseId": "%SRCROOT%",
|
||||
},
|
||||
"region": {"startLine": line},
|
||||
"region": region,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -970,22 +1078,36 @@ def sarif_result(
|
|||
)
|
||||
if finding["verdict"] == "UNVERIFIED":
|
||||
message += "\n\n" + SARIF_UNVERIFIED_NOTE
|
||||
location = finding["location"]
|
||||
start_line = location["start_line"]
|
||||
end_line = location["end_line"]
|
||||
fingerprint_anchor = (
|
||||
location["existing_code"]
|
||||
or f"{start_line}:{end_line}"
|
||||
)
|
||||
fingerprint = hashlib.sha256(
|
||||
(
|
||||
f"{finding['file']}\0{finding['issue_type']}\0"
|
||||
f"{fingerprint_anchor}"
|
||||
).encode("utf-8")
|
||||
).hexdigest()
|
||||
result: Dict[str, Any] = {
|
||||
"ruleId": primary,
|
||||
"ruleIndex": rule_indices[primary],
|
||||
"level": SARIF_LEVELS[finding["severity"]],
|
||||
"message": {"text": message},
|
||||
"locations": [sarif_location(finding["file"], finding["line"])],
|
||||
"locations": [
|
||||
sarif_location(finding["file"], start_line, end_line)
|
||||
],
|
||||
"partialFingerprints": {
|
||||
"codeReviewIdentity/v1": (
|
||||
f"{finding['file']}:{finding['line']}:{finding['category']}"
|
||||
)
|
||||
"codeReviewIdentity/v2": fingerprint
|
||||
},
|
||||
"properties": {
|
||||
key: finding[key]
|
||||
for key in (
|
||||
"id",
|
||||
"category",
|
||||
"issue_type",
|
||||
"severity",
|
||||
"confidence",
|
||||
"verdict",
|
||||
|
|
@ -997,6 +1119,32 @@ def sarif_result(
|
|||
)
|
||||
},
|
||||
}
|
||||
suggestion = finding.get("suggestion")
|
||||
if isinstance(suggestion, dict):
|
||||
result["fixes"] = [
|
||||
{
|
||||
"description": {"text": "Apply the verified suggested change"},
|
||||
"artifactChanges": [
|
||||
{
|
||||
"artifactLocation": {
|
||||
"uri": sarif_uri(finding["file"]),
|
||||
"uriBaseId": "%SRCROOT%",
|
||||
},
|
||||
"replacements": [
|
||||
{
|
||||
"deletedRegion": {
|
||||
"startLine": start_line,
|
||||
"endLine": end_line,
|
||||
},
|
||||
"insertedContent": {
|
||||
"text": suggestion["replacement_code"]
|
||||
},
|
||||
}
|
||||
],
|
||||
}
|
||||
],
|
||||
}
|
||||
]
|
||||
anchors = finding.get("anchors") or []
|
||||
if anchors:
|
||||
result["relatedLocations"] = [
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ report.
|
|||
|
||||
## Canonical files
|
||||
|
||||
The canonical bundle is schema version 3.
|
||||
The canonical bundle is schema version 4.
|
||||
|
||||
- `review-manifest.json` identifies the review, target, revision, request,
|
||||
completion status, counts, and canonical file set. At the rule-mapped
|
||||
|
|
@ -22,8 +22,10 @@ The canonical bundle is schema version 3.
|
|||
carries the candidate's applicable `rule_ids` (empty outside the
|
||||
rule-mapped tiers).
|
||||
- `findings.json` contains only the reportable subset. It is the authoritative
|
||||
finding list. Each reported finding also carries a `code` excerpt, which the
|
||||
ledger's candidate records do not, and its `rule_ids`.
|
||||
finding list. Each reported finding carries its orthogonal `issue_type`, an
|
||||
engine-derived `location` with the exact original code and start/end lines,
|
||||
a highlighted `code` excerpt, and its `rule_ids`. A verified replacement is
|
||||
stored as optional `suggestion.replacement_code`.
|
||||
- `coverage.json` records what the review dispatched, what returned, what was
|
||||
rejected for failing the finding contract, and what a cap dropped. At the
|
||||
rule-mapped tiers it also records the authoritative target-file list, the
|
||||
|
|
@ -39,9 +41,10 @@ The canonical bundle is schema version 3.
|
|||
and cap drops -- also emitted into the workflow context so calibration
|
||||
across many runs can read it from the event log.
|
||||
- `votes.jsonl` contains one record for each dispatched verification, with the
|
||||
exact claim shown to the verifier (including its claimed `rule_ids` and the
|
||||
file's effective checks at the rule-mapped tiers), plus its verdict and
|
||||
reasoning when it completed.
|
||||
exact claim shown to the verifier (including its location, proposed
|
||||
replacement, claimed `rule_ids`, and the file's effective checks at the
|
||||
rule-mapped tiers), plus its verdict and reasoning when it completed. A vote
|
||||
over a proposed replacement also carries `suggestion_valid`.
|
||||
|
||||
## Derived files
|
||||
|
||||
|
|
@ -117,6 +120,12 @@ verifier's instructions, never the arithmetic. At `low`, verification is
|
|||
skipped by design: findings carry `verdict: "UNVERIFIED"` and the reports say
|
||||
so.
|
||||
|
||||
A proposed replacement is independent of the keep verdict. The verifier must
|
||||
return `suggestion_valid: true`, the engine must be able to read the exact
|
||||
original range from the unchanged reviewed tree, and the replacement must
|
||||
differ from it. Low-effort findings never carry suggestions because they have
|
||||
no independent verification.
|
||||
|
||||
## Deduplication and ranking
|
||||
|
||||
A candidate's identity is its normalized file, line, and category. Two angles
|
||||
|
|
@ -146,12 +155,16 @@ categories (`reuse`, `simplification`, `efficiency`, `altitude`,
|
|||
report count, then confidence, then file and line. The report cap cuts from
|
||||
the bottom, and everything cut is in the ledger as `deferred-by-cap`.
|
||||
|
||||
## Source excerpts
|
||||
## Locations, source excerpts, and suggestions
|
||||
|
||||
A finding's `code` excerpt is not agent-quoted: `final-tally` reads the lines
|
||||
around the finding from the reviewed tree, so the line numbers are the tree's
|
||||
own and no agent transcribes them. The excerpt is omitted when the file is
|
||||
unreadable, binary, oversized, or the line is out of range.
|
||||
A finder supplies a bounded `start_line`/`end_line` range. `final-tally` reads
|
||||
that range from the reviewed tree and records its exact text as
|
||||
`location.existing_code`; the agent never supplies the canonical original
|
||||
text. The adjacent `code` excerpt is read the same way and highlights the
|
||||
complete range. Exact text and the excerpt are omitted when the file is
|
||||
unreadable, binary, oversized, or the range is invalid. A proposed
|
||||
`suggestion_code` becomes canonical only after the verifier approves it and
|
||||
the exact original text is available.
|
||||
|
||||
## HTML rendering
|
||||
|
||||
|
|
@ -174,11 +187,14 @@ validated bundle:
|
|||
covers every check ID any result cites.
|
||||
- Severity maps to level: `HIGH` is `error`, `MEDIUM` is `warning`, `LOW` is
|
||||
`note`.
|
||||
- Each result's location is the finding's file and line relative to
|
||||
- Each result's location is the finding's file and line range relative to
|
||||
`%SRCROOT%`; anchors become related locations. The finding's identity,
|
||||
category, severity, confidence, verdict, reports, reporters, rule IDs,
|
||||
anchors, and source are result properties, and the file, line, and
|
||||
category form a stable partial fingerprint.
|
||||
category, issue type, severity, confidence, verdict, reports, reporters,
|
||||
rule IDs, anchors, and source are result properties. File, issue type, and
|
||||
exact original code form a stable hashed partial fingerprint, falling back
|
||||
to the line range when source text is unavailable.
|
||||
- A verified suggestion becomes a SARIF `fix` that replaces the complete
|
||||
location range.
|
||||
- An `UNVERIFIED` finding (the `low` tier) says so in its result message and
|
||||
carries the verdict in its properties.
|
||||
- The run's automation ID is `code-review/<mode>`, and the run properties
|
||||
|
|
|
|||
|
|
@ -203,6 +203,19 @@ function renderCode(code) {
|
|||
return details;
|
||||
}
|
||||
|
||||
function renderSuggestedChange(finding) {
|
||||
if (!finding.suggestion || !finding.location ||
|
||||
!finding.location.existing_code) return null;
|
||||
const details = el("details");
|
||||
details.appendChild(el("summary", null, "Suggested change"));
|
||||
details.appendChild(el("p", "label", "Before"));
|
||||
details.appendChild(el("pre", "code", finding.location.existing_code));
|
||||
details.appendChild(el("p", "label", "After"));
|
||||
details.appendChild(el("pre", "code",
|
||||
finding.suggestion.replacement_code || ""));
|
||||
return details;
|
||||
}
|
||||
|
||||
function renderFinding(finding) {
|
||||
const card = el("article", "finding");
|
||||
const head = el("div", "finding-head");
|
||||
|
|
@ -213,6 +226,7 @@ function renderFinding(finding) {
|
|||
chips.appendChild(el("span", "chip sev-" + finding.severity,
|
||||
finding.severity + " severity"));
|
||||
chips.appendChild(el("span", "chip", finding.category));
|
||||
chips.appendChild(el("span", "chip", finding.issue_type));
|
||||
if ((finding.rule_ids || []).length) {
|
||||
chips.appendChild(el("span", "chip",
|
||||
"rule " + finding.rule_ids.join(", ")));
|
||||
|
|
@ -222,7 +236,11 @@ function renderFinding(finding) {
|
|||
chips.appendChild(el("span", "chip",
|
||||
finding.reports + " report(s): " + (finding.reporters || []).join(", ")));
|
||||
card.appendChild(chips);
|
||||
card.appendChild(el("p", "location", finding.file + ":" + finding.line));
|
||||
const loc = finding.location || {};
|
||||
const span = loc.start_line && loc.end_line && loc.start_line !== loc.end_line
|
||||
? loc.start_line + "-" + loc.end_line
|
||||
: (loc.end_line || finding.line);
|
||||
card.appendChild(el("p", "location", finding.file + ":" + span));
|
||||
if ((finding.anchors || []).length) {
|
||||
card.appendChild(el("p", "location", "Also reported at " +
|
||||
finding.anchors.map(a => a.file + ":" + a.line + " (" + a.category +
|
||||
|
|
@ -242,6 +260,8 @@ function renderFinding(finding) {
|
|||
verifier.appendChild(el("p", null, finding.verdict_reasoning));
|
||||
card.appendChild(verifier);
|
||||
}
|
||||
const suggestion = renderSuggestedChange(finding);
|
||||
if (suggestion) card.appendChild(suggestion);
|
||||
const code = renderCode(finding.code);
|
||||
if (code) card.appendChild(code);
|
||||
return card;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue