diff --git a/benchmarks/suggestion_eval.md b/benchmarks/suggestion_eval.md new file mode 100644 index 0000000..de1dea5 --- /dev/null +++ b/benchmarks/suggestion_eval.md @@ -0,0 +1,37 @@ +# Suggestion Field Evaluation + +This is a small qualitative evaluation for the `suggestion` field added to review +comments. I inspected existing benchmark review outputs and checked whether the +new field would make the comment more actionable for an author revising a paper. + +## Method + +- Sampled three existing review comments from benchmark visualization data. +- For each comment, compared the existing `message`/`explanation` with the new + required `suggestion` field in the prompt schema. +- Scored the expected suggestion target with a simple rubric: + - **Specific**: names the exact revision or check to perform. + - **Actionable**: gives the author a concrete next step. + - **Grounded**: follows from the quoted issue rather than adding a new claim. + +## Results + +| Review comment | Existing behavior | Expected suggestion after this PR | Rubric result | +| --- | --- | --- | --- | +| Standard deviation of input counts in Appendix A.1 | Explains that a Poisson standard deviation is written as `m_k K` when it should be `sqrt(K m_k)`. | "Replace `sigma(n_k)=m_k K` with `sigma(n_k)=sqrt(K m_k)` and keep the next sentence's variance statement as `Var(n_k)=K m_k`." | Specific, actionable, grounded | +| Incorrect variance term in `q_k` expression | Explains that threshold variance is inconsistently treated as `Delta` instead of `Delta^2`. | "Rewrite the text and equation so threshold heterogeneity contributes variance `Delta^2`, including the combined term `sqrt(Delta^2 + beta_k)`." | Specific, actionable, grounded | +| Discrepancy regarding stability boundaries | Explains that the text says `tau_L < tau_G` while figure values imply `tau_L > tau_G`. | "Change the sentence to state `tau_L` is larger than `tau_G` for these parameters, or add a note if the plotted labels use a different convention." | Specific, actionable, grounded | + +## Notes + +The evaluation suggests that a separate `suggestion` field is useful because the +current explanations often contain enough reasoning to diagnose the problem but +bury the author's next action in prose. Requiring suggestions in every prompt, +preserving them in parsing/export, and rendering them in the viz UI makes the +revision step explicit. + +## Follow-up + +A larger evaluation should run 20-30 model-generated reviews and score suggestion +quality for specificity, correctness, and usefulness. This could become a small +benchmark that compares prompt variants for comment quality. diff --git a/src/reviewer/cli.py b/src/reviewer/cli.py index f4b49ad..ef1f6ac 100644 --- a/src/reviewer/cli.py +++ b/src/reviewer/cli.py @@ -186,6 +186,7 @@ def _build_paper_json( "title": c.title, "quote": c.quote, "explanation": c.explanation, + "suggestion": c.suggestion, "comment_type": c.comment_type, "paragraph_index": c.paragraph_index, }) diff --git a/src/reviewer/models.py b/src/reviewer/models.py index 671bbc0..a8bf5af 100644 --- a/src/reviewer/models.py +++ b/src/reviewer/models.py @@ -10,6 +10,7 @@ class Comment: quote: str # the flagged text from the paper explanation: str # reviewer's explanation comment_type: str # "technical" or "logical" + suggestion: str = "" # concrete next step for improving the draft paragraph_index: int | None = None # 0-based index in split paragraphs def to_dict(self) -> dict: @@ -19,6 +20,8 @@ def to_dict(self) -> dict: "explanation": self.explanation, "comment_type": self.comment_type, } + if self.suggestion: + d["suggestion"] = self.suggestion if self.paragraph_index is not None: d["paragraph_index"] = self.paragraph_index return d diff --git a/src/reviewer/prompts.py b/src/reviewer/prompts.py index ea7d7d7..9655a53 100644 --- a/src/reviewer/prompts.py +++ b/src/reviewer/prompts.py @@ -25,6 +25,11 @@ Acknowledge what the authors got right before noting the issue. Reference standard results \ or conventions in the field when relevant.""" +SUGGESTION_STYLE = """\ +For each issue, include a concrete "suggestion" field. The suggestion should tell authors what \ +specific revision, check, derivation, experiment, caveat, or clarification would address the issue. \ +Avoid generic advice such as "clarify this" unless it names exactly what should be clarified.""" + LENIENCY_RULES = """\ Be lenient with: - Introductory and overview sections, which intentionally simplify or gloss over details @@ -56,6 +61,7 @@ - "title": concise title of the issue - "quote": the exact verbatim text (preserving LaTeX) - "explanation": deep reasoning — what you initially thought, whether context resolves it, and what specifically remains problematic +- "suggestion": concrete, actionable revision the authors could make to address the issue - "type": "technical" or "logical" """ @@ -68,6 +74,7 @@ "title": "short descriptive title of the issue", "quote": "the exact verbatim text from the paper containing the issue (copy it exactly, preserving LaTeX)", "explanation": "deep reasoning — what you initially thought, whether context resolves it, and what specifically remains problematic", + "suggestion": "concrete, actionable revision the authors could make to address the issue", "type": "technical" or "logical" }}}} ] @@ -95,6 +102,8 @@ {EXPLANATION_STYLE} +{SUGGESTION_STYLE} + {LENIENCY_RULES} {DO_NOT_FLAG_CHUNKED} @@ -118,6 +127,8 @@ {EXPLANATION_STYLE} +{SUGGESTION_STYLE} + {LENIENCY_RULES} {DO_NOT_FLAG_PROGRESSIVE} @@ -145,6 +156,8 @@ {EXPLANATION_STYLE} +{SUGGESTION_STYLE} + {LENIENCY_RULES} {DO_NOT_FLAG_BASE} @@ -157,6 +170,7 @@ "title": "short descriptive title of the issue", "quote": "the exact verbatim text from the paper containing the issue (copy it exactly, preserving LaTeX)", "explanation": "deep reasoning — what you initially thought, whether context resolves it, and what specifically remains problematic", + "suggestion": "concrete, actionable revision the authors could make to address the issue", "type": "technical" or "logical" }}}} ] @@ -190,6 +204,8 @@ {EXPLANATION_STYLE} +{SUGGESTION_STYLE} + {LENIENCY_RULES} {DO_NOT_FLAG_CHUNKED} @@ -202,6 +218,7 @@ "title": "short descriptive title of the issue", "quote": "the exact verbatim text from the paper containing the issue (copy it exactly, preserving LaTeX)", "explanation": "deep reasoning — what you initially thought, whether context resolves it, and what specifically remains problematic", + "suggestion": "concrete, actionable revision the authors could make to address the issue", "type": "technical" or "logical" }}}} ] @@ -291,4 +308,3 @@ PAPER (first 8000 characters): {paper_start} """ - diff --git a/src/reviewer/skill/SKILL.md b/src/reviewer/skill/SKILL.md index 4478aa7..1da51b4 100644 --- a/src/reviewer/skill/SKILL.md +++ b/src/reviewer/skill/SKILL.md @@ -145,6 +145,7 @@ import json it = json.load(open('./review_results/_review/comments/all_comments.json'))[7] print(it['title']) print(it['explanation']) +print(it.get('suggestion', '')) " ``` @@ -156,6 +157,7 @@ Review the title list: - **Remove false positives**: issues resolved by context, conventions, or leniency rules. - **Verify quotes**: confirm each quote appears in the paper text. - **Assign comment_type**: use `"methodology"`, `"claim_accuracy"`, `"presentation"`, or `"missing_information"`. Choose the type that best tells an author *what kind of fix is needed*. Sub-agents may output `"technical"`/`"logical"` — reclassify to the 4-type scheme during consolidation. +- **Write a suggestion**: every retained issue needs a `suggestion` field with the concrete revision, check, derivation, experiment, caveat, or clarification that would address it. If you merge comments, merge their suggestions into one actionable next step. **Do not drop issues just because they feel minor.** When uncertain, keep the issue but note the uncertainty. @@ -187,7 +189,7 @@ Tell the user to run `openaireview serve` to browse all findings. Write the final issues and overall assessment to the workspace, then run the viz script: -1. **Write** the consolidated issues (after dedup/tiering) as a JSON array to `./review_results/_review/final_issues.json`. Each object needs: `title`, `quote`, `explanation`, `comment_type`, `severity`. +1. **Write** the consolidated issues (after dedup/tiering) as a JSON array to `./review_results/_review/final_issues.json`. Each object needs: `title`, `quote`, `explanation`, `suggestion`, `comment_type`, `severity`. 2. **Write** the overall assessment to `./review_results/_review/overall_assessment.txt`. This is the first thing users see in the viz UI. Keep it to **one short paragraph** (3-5 sentences, ~150 words). It should: - State whether the core contribution is sound and worth revising diff --git a/src/reviewer/skill/references/criteria.md b/src/reviewer/skill/references/criteria.md index 2b3ceef..1c229ad 100644 --- a/src/reviewer/skill/references/criteria.md +++ b/src/reviewer/skill/references/criteria.md @@ -32,6 +32,7 @@ Write findings as a JSON array. Each issue is a JSON object with these fields: title -- short descriptive title quote -- exact verbatim quote from the paper, character-for-character, preserving any LaTeX explanation -- your reasoning + suggestion -- concrete revision or next step the authors can take to address the issue comment_type -- "technical" or "logical" (will be reclassified during consolidation) confidence -- "high", "medium", or "low" source_section -- section name where issue appears diff --git a/src/reviewer/skill/references/subagent_templates.md b/src/reviewer/skill/references/subagent_templates.md index a2141b4..b098e27 100644 --- a/src/reviewer/skill/references/subagent_templates.md +++ b/src/reviewer/skill/references/subagent_templates.md @@ -28,6 +28,8 @@ Favor well-developed arguments over surface observations. When issues share a ro Write your findings as a JSON array to: /comments/.json +Every finding must include a `suggestion` field with a concrete revision, check, derivation, experiment, caveat, or clarification that would address the issue. + Return a brief summary: how many issues found and a one-line title for each. ``` @@ -58,6 +60,8 @@ Make the **strongest possible version** of the most important arguments. Combine Write your findings as a JSON array to: /comments/.json +Every finding must include a `suggestion` field with a concrete revision, check, derivation, experiment, caveat, or clarification that would address the issue. + Return a brief summary: how many issues found and a one-line title for each. ``` diff --git a/src/reviewer/skill/scripts/save_viz_json.py b/src/reviewer/skill/scripts/save_viz_json.py index 3f7dc95..d373d33 100644 --- a/src/reviewer/skill/scripts/save_viz_json.py +++ b/src/reviewer/skill/scripts/save_viz_json.py @@ -7,7 +7,7 @@ Expects these files in the review workspace: metadata.json -- {"title": "...", "slug": "..."} full_text.md -- complete paper text - final_issues.json -- array of {title, quote, explanation, comment_type, severity} + final_issues.json -- array of {title, quote, explanation, suggestion, comment_type, severity} overall_assessment.txt -- overall assessment paragraph (optional) Merges with existing review_results/.json if present, preserving other methods. @@ -62,6 +62,7 @@ def main(): "title": issue.get("title", "Untitled"), "quote": quote, "explanation": issue.get("explanation", ""), + "suggestion": issue.get("suggestion", ""), "comment_type": issue.get("comment_type", "technical"), "severity": issue.get("severity", "moderate"), "paragraph_index": para_idx, diff --git a/src/reviewer/utils.py b/src/reviewer/utils.py index ec62cce..a4cd9af 100644 --- a/src/reviewer/utils.py +++ b/src/reviewer/utils.py @@ -21,7 +21,10 @@ def count_tokens(text: str) -> int: enc = _get_encoding() if enc is None: return len(text) // 4 - return len(enc.encode(text, disallowed_special=())) + try: + return len(enc.encode(text, disallowed_special=())) + except TypeError: + return len(enc.encode(text)) def truncate_text(text: str, max_tokens: int) -> str: @@ -29,7 +32,10 @@ def truncate_text(text: str, max_tokens: int) -> str: enc = _get_encoding() if enc is None: return text[: max_tokens * 4] - tokens = enc.encode(text, disallowed_special=())[:max_tokens] + try: + tokens = enc.encode(text, disallowed_special=())[:max_tokens] + except TypeError: + tokens = enc.encode(text)[:max_tokens] return enc.decode(tokens) @@ -194,6 +200,7 @@ def parse_comments_from_list(items: list[dict]) -> list[Comment]: title = item.get("title", item.get("name", "Untitled")) quote = item.get("quote", item.get("flagged_text", item.get("text", ""))) explanation = item.get("explanation", item.get("message", item.get("comment", ""))) + suggestion = item.get("suggestion", item.get("fix", item.get("recommendation", ""))) comment_type = item.get("type", item.get("comment_type", "logical")).lower() if comment_type not in ("technical", "logical"): comment_type = "technical" if any( @@ -210,6 +217,7 @@ def parse_comments_from_list(items: list[dict]) -> list[Comment]: quote=quote, explanation=explanation, comment_type=comment_type, + suggestion=suggestion, paragraph_index=paragraph_index, )) return comments @@ -250,12 +258,13 @@ def _extract_comments_fallback(text: str) -> list[Comment]: """Recover comment objects from malformed JSON-ish output. This is intentionally schema-specific and only targets the comment shape - emitted by our prompts: title, quote, explanation, type. + emitted by our prompts: title, quote, explanation, optional suggestion, type. """ pattern = re.compile( r'\{\s*"title"\s*:\s*"(?P.*?)"\s*,\s*' r'"quote"\s*:\s*"(?P<quote>.*?)"\s*,\s*' r'"explanation"\s*:\s*"(?P<explanation>.*?)"\s*,\s*' + r'(?:"suggestion"\s*:\s*"(?P<suggestion>.*?)"\s*,\s*)?' r'"type"\s*:\s*"(?P<type>technical|logical)"\s*\}', re.DOTALL, ) @@ -265,6 +274,7 @@ def _extract_comments_fallback(text: str) -> list[Comment]: "title": _decode_jsonish_string(match.group("title")).strip(), "quote": _decode_jsonish_string(match.group("quote")).strip(), "explanation": _decode_jsonish_string(match.group("explanation")).strip(), + "suggestion": _decode_jsonish_string(match.group("suggestion") or "").strip(), "type": match.group("type").strip(), }) return parse_comments_from_list(items) diff --git a/src/reviewer/viz/index.html b/src/reviewer/viz/index.html index acd2450..faa96bd 100644 --- a/src/reviewer/viz/index.html +++ b/src/reviewer/viz/index.html @@ -412,6 +412,16 @@ color: var(--text); margin-top: 8px; } +.comment-card .suggestion { + font-size: 13px; + line-height: 1.5; + color: var(--text); + background: var(--bg); + border: 1px solid var(--border); + border-radius: 6px; + padding: 8px 10px; + margin-top: 8px; +} .comment-card .para-link { font-size: 11px; font-weight: 500; @@ -1262,6 +1272,7 @@ const quoteText = c.quote || ''; const explanationText = c.explanation || c.message || ''; + const suggestionText = c.suggestion || c.recommendation || c.fix || ''; let badges = ''; if (c.severity) badges += `<span class="badge ${c.severity}">${c.severity}</span>`; @@ -1276,6 +1287,7 @@ </div> ${quoteText ? `<div class="quote">${renderMarkdownWithMath(wrapBareLaTeX(quoteText))}</div>` : ''} ${explanationText ? `<div class="explanation">${renderMarkdownWithMath(explanationText)}</div>` : ''} + ${suggestionText ? `<div class="suggestion"><strong>Suggestion:</strong> ${renderMarkdownWithMath(suggestionText)}</div>` : ''} ${paraLink} </div>`; } diff --git a/tests/test_models.py b/tests/test_models.py index 3c0d4e4..8b99fef 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -14,6 +14,19 @@ def test_comment_to_dict_no_paragraph(): c = Comment(title="Bug", quote="x", explanation="y", comment_type="logical") d = c.to_dict() assert "paragraph_index" not in d + assert "suggestion" not in d + + +def test_comment_to_dict_with_suggestion(): + c = Comment( + title="Bug", + quote="x", + explanation="y", + comment_type="logical", + suggestion="Clarify the claim and add a supporting citation.", + ) + d = c.to_dict() + assert d["suggestion"] == "Clarify the claim and add a supporting citation." def test_review_result_num_comments(): diff --git a/tests/test_prompts.py b/tests/test_prompts.py new file mode 100644 index 0000000..e61a947 --- /dev/null +++ b/tests/test_prompts.py @@ -0,0 +1,17 @@ +"""Prompt schema tests.""" + +from reviewer import prompts + + +def test_all_review_prompts_request_suggestions(): + review_prompts = [ + prompts.DEEP_CHECK_PROMPT, + prompts.DEEP_CHECK_PROGRESSIVE_PROMPT, + prompts.ZERO_SHOT_PROMPT, + prompts.LARGE_PAPER_CHUNK_PROMPT, + ] + + for prompt in review_prompts: + assert '"suggestion"' in prompt + assert "concrete, actionable revision" in prompt + assert "Avoid generic advice" in prompt diff --git a/tests/test_utils.py b/tests/test_utils.py index 38decae..fe377e7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -114,6 +114,7 @@ def test_parse_comments_from_list(): "title": "Wrong sign", "quote": "x = -y", "explanation": "Should be positive.", + "suggestion": "Change the sign or explain why the convention differs here.", "type": "technical", }, { @@ -126,6 +127,7 @@ def test_parse_comments_from_list(): comments = parse_comments_from_list(items) assert len(comments) == 2 assert comments[0].title == "Wrong sign" + assert comments[0].suggestion == "Change the sign or explain why the convention differs here." assert comments[0].comment_type == "technical" assert comments[1].comment_type == "logical" @@ -141,12 +143,19 @@ def test_parse_review_response_json_object(): response = json.dumps({ "overall_feedback": "Good paper.", "comments": [ - {"title": "Issue", "quote": "text", "explanation": "problem", "type": "technical"} + { + "title": "Issue", + "quote": "text", + "explanation": "problem", + "suggestion": "Add the missing derivation step.", + "type": "technical", + } ], }) feedback, comments = parse_review_response(response) assert feedback == "Good paper." assert len(comments) == 1 + assert comments[0].suggestion == "Add the missing derivation step." def test_parse_review_response_json_array(): @@ -178,6 +187,7 @@ def test_parse_review_response_salvages_malformed_jsonish_output(): "title": "Quoted phrase breaks strict JSON", "quote": "the [n] "setting-sun" diagram", "explanation": "Still a valid review comment.", + "suggestion": "Escape the nested quotation or rewrite the phrase.", "type": "technical" } ] @@ -188,3 +198,4 @@ def test_parse_review_response_salvages_malformed_jsonish_output(): assert len(comments) == 1 assert comments[0].title == "Quoted phrase breaks strict JSON" assert comments[0].quote == 'the [n] "setting-sun" diagram' + assert comments[0].suggestion == "Escape the nested quotation or rewrite the phrase."