Skip to content

Add optional suggested fixes to review comments#94

Open
sahilagr123 wants to merge 1 commit into
ChicagoHAI:mainfrom
sahilagr123:suggested-fix-comments
Open

Add optional suggested fixes to review comments#94
sahilagr123 wants to merge 1 commit into
ChicagoHAI:mainfrom
sahilagr123:suggested-fix-comments

Conversation

@sahilagr123
Copy link
Copy Markdown

Summary

This PR adds an optional suggested_fix field to review comments.

The motivation is issue #31: some comments point out a problem, but do not always give the author a concrete next step. This also connects to the interaction part of the roadmap, since comments are more useful if authors can actually act on them.

What changed

  • Added suggested_fix to the Comment model.
  • Kept it optional and omitted it from JSON when empty.
  • Updated comment parsing so old outputs without this field still work.
  • Updated prompts to ask for a concrete fix when there is one, and to leave it empty instead of giving vague advice.
  • Preserved suggested_fix through progressive consolidation.
  • Added it to the visualization JSON when present.
  • Added a small “Suggested fix” block in the UI.
  • Added tests for serialization and parsing.

Out of scope

I kept this limited to the main CLI / Comment pipeline. I did not update the Claude Code skill pipeline, benchmarks, provider code, OCR code, or add any new dependencies.

The malformed-JSON fallback parser also does not try to recover suggested_fix, it will just default to empty there.

Testing

Ran:

.venv/bin/python -m pytest tests/test_models.py tests/test_utils.py tests/test_progressive.py -q

Results: 27 passed, 1 failed.

The failing test appears unrelated to this PR: test_count_tokens_falls_back_when_model_encoding_init_fails fails because the fake tokenizer in the test does not accept the disallowed_special argument.

I also manually tested the UI by temporarily adding a suggested_fix to an old local review result JSON and running openaireview serve. The “Suggested fix” block rendered correctly, hid when the comment was resolved, and comments without the field looked unchanged. That temporary JSON edit is not included in this PR.

Please let me know what you think, and if there is anything to change or revise. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant