A tale of two fixes#24
Conversation
shellygr
commented
Jun 28, 2026
- For code without user-defined data types, the summary resolver would crash since it would send an empty block to the LLM. Handle this case to avoid it triggering. Point to consider: make a more global fix in graphcore?
- Autosetup failed on a second run on the same project when parsing *_call_resolution.spec due to ASTExtraction output mishandling, trying to parse non-json as json. Context: It is part of the code that comments out irrelevant auto-injected summaries.
… deal with Warnings and Errors prepended before the actual json in stdout. We drop the non-json messages and handle with proper logging any remaining json decode errors
The pure _parse_ast_payload parser is unit-tested in the cheap pytest CI job, which installs deps without the prover extra (no certora_cli). The module-level `from certora_cli... import find_jar` made importing the module — and thus collecting tests/test_summary_resolver.py — fail with ModuleNotFoundError. find_jar is the only certora_cli symbol in this module's import chain, so defer it into _ast_extraction (matching existing lazy-import patterns in the package). The module now imports without certora_cli; the parser test runs in fast CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cheap pytest job synced deps without certora_cli, so collecting tests/test_summary_resolver.py (which imports certora_autosetup.setup. summary_resolver -> certora_cli find_jar) failed with ModuleNotFoundError. Provide the dependency in CI (`uv sync --group test --extra prover`) rather than contorting the module to import without it. Reverts the lazy-import change from 17953ef; summary_resolver keeps its top-level find_jar import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jtoman
left a comment
There was a problem hiding this comment.
approving the AIC parts, leaving AS parts to my betters
| curr_spec=None, | ||
| input=[ | ||
| "The following types are available for use in your spec", | ||
| udts, |
There was a problem hiding this comment.
I know that sending an empty string as the entire message payload could cause an error, but an empty string in the messages array is a new one. agreed on the fix at the graphcore level, something we can fold into the "invoke" wrappers I've added in Certora/graphcore#21
There was a problem hiding this comment.
have you applied this in graphcore#21? it seems not?
There was a problem hiding this comment.
not yet, since I didn't want to make this PR rely on a fix that hasn't landed yet. But there's no reason not to do it now anyway...
naftali-g
left a comment
There was a problem hiding this comment.
a nit and a comment, but approving to not block
| except json.JSONDecodeError as exc: | ||
| raise RuntimeError( | ||
| f"{_AST_EXTRACTION_JAR} output was not JSON after stripping diagnostics " | ||
| f"(exit {returncode}): {exc}; stdout_head={stdout[:300]!r} stderr={stderr.strip()[:500]!r}" |
There was a problem hiding this comment.
just print all stdout and stderr, no?
Co-authored-by: Naftali Goldstein <44599898+naftali-g@users.noreply.github.com>
…a/AutoProver into shelly/autoprover-summaries-fixes