Feat/phase4 - #5
Conversation
DSCmatter
added
documentation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:XXL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| if e_field == field_path and (run_id is None or not e_run or e_run == run_id): | ||
| results.append(e) |
There was a problem hiding this comment.
Suggestion: When a run-specific query is requested, edges without a run_id are still returned because not e_run evaluates true. This allows unscoped provenance edges to appear in an unrelated run, contaminating provenance results across run boundaries. Require an exact run ID match whenever run_id is provided. [security]
Severity Level: Major ⚠️
- ⚠️ In-memory provenance chains can cross run boundaries.
- ⚠️ Debugging and audit results may include unrelated tool outputs.
- ⚠️ Postgres storage avoids this through its required run identifier.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** sdk/events.py
**Line:** 246:247
**Comment:**
*Security: When a run-specific query is requested, edges without a `run_id` are still returned because `not e_run` evaluates true. This allows unscoped provenance edges to appear in an unrelated run, contaminating provenance results across run boundaries. Require an exact run ID match whenever `run_id` is provided.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| # Record exact field-level provenance if configured | ||
| if field_sources is not None: | ||
| if isinstance(field_sources, dict): | ||
| sources = field_sources | ||
| else: | ||
| sources = field_sources(result) | ||
| if sources: | ||
| from core.provenance import record_tool_result_provenance | ||
|
|
||
| record_tool_result_provenance( | ||
| result_event, | ||
| sources, | ||
| store=log, | ||
| transform=transform or "tool_call", | ||
| ) |
There was a problem hiding this comment.
Suggestion: Persisting provenance after the tool_result event makes provenance failure observable as a tool-call failure even though the external tool result is already committed. With a backend such as Postgres, an invalid or missing run_id can make record_provenance_edge raise after the result is stored; subsequent retries find the existing result and return early without retrying the missing provenance writes. Persist provenance atomically with the result or make provenance recording best-effort and retryable. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Successful tool calls can be reported as failures.
- ⚠️ Retried invocations can permanently lack provenance edges.
- ⚠️ Provenance audits become incomplete after transient backend failures.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** sdk/tools.py
**Line:** 140:154
**Comment:**
*Incomplete Implementation: Persisting provenance after the `tool_result` event makes provenance failure observable as a tool-call failure even though the external tool result is already committed. With a backend such as Postgres, an invalid or missing `run_id` can make `record_provenance_edge` raise after the result is stored; subsequent retries find the existing result and return early without retrying the missing provenance writes. Persist provenance atomically with the result or make provenance recording best-effort and retryable.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| def record_provenance_edge(self, edge: ProvenanceEdge) -> ProvenanceEdge: | ||
| edge_id = self._uuid(edge.id, "ProvenanceEdge.id") | ||
| run_id = self._uuid(edge.run_id, "ProvenanceEdge.run_id") | ||
| source_event_id = self._uuid(edge.source_event_id, "ProvenanceEdge.source_event_id") |
There was a problem hiding this comment.
Suggestion: The insert validates that run_id and source_event_id are individually valid UUIDs but does not verify that the referenced source event belongs to the same run. PostgreSQL's separate foreign key permits an edge for one run to point at an event from another run, corrupting provenance ownership and enabling cross-run attribution. Validate the event's run ID or enforce the relationship with a composite constraint. [security]
Severity Level: Major ⚠️
- ❌ Persisted provenance ownership can be corrupted.
- ⚠️ Cross-run source attribution undermines audit accuracy.
- ⚠️ Direct edge persistence accepts inconsistent run relationships.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** storage/postgres.py
**Line:** 532:535
**Comment:**
*Security: The insert validates that `run_id` and `source_event_id` are individually valid UUIDs but does not verify that the referenced source event belongs to the same run. PostgreSQL's separate foreign key permits an edge for one run to point at an event from another run, corrupting provenance ownership and enabling cross-run attribution. Validate the event's run ID or enforce the relationship with a composite constraint.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| with self.connection.cursor() as cursor: | ||
| cursor.execute( | ||
| sql, | ||
| ( | ||
| edge_id, | ||
| run_id, | ||
| edge.field_path, | ||
| source_event_id, | ||
| edge.source_path, | ||
| edge.grade, | ||
| edge.transform, | ||
| edge.created_at, | ||
| ), | ||
| ) | ||
| row = cursor.fetchone() | ||
| self.connection.commit() |
There was a problem hiding this comment.
Suggestion: Exceptions from the insert or commit leave the shared psycopg connection in an aborted transaction state because this method does not roll back on failure. A foreign-key, enum, or other database error will therefore cause subsequent operations using the same connection to fail until another caller explicitly rolls it back. Wrap the cursor execution and commit in the same rollback-on-exception pattern used by append and save. [missing cleanup]
Severity Level: Major ⚠️
- ❌ Subsequent database operations fail after rejected provenance writes.
- ⚠️ CLI provenance and event workflows share the affected connection.
- ⚠️ Connections require external rollback before reuse.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** storage/postgres.py
**Line:** 546:561
**Comment:**
*Missing Cleanup: Exceptions from the insert or commit leave the shared psycopg connection in an aborted transaction state because this method does not roll back on failure. A foreign-key, enum, or other database error will therefore cause subsequent operations using the same connection to fail until another caller explicitly rolls it back. Wrap the cursor execution and commit in the same rollback-on-exception pattern used by `append` and `save`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| with self.connection.cursor() as cursor: | ||
| if run_id is None: | ||
| cursor.execute( | ||
| "SELECT id, run_id, field_path, source_event_id, source_path, grade, " | ||
| "transform, created_at FROM provenance_edges " | ||
| "WHERE field_path = %s ORDER BY created_at ASC", | ||
| (field_path,), |
There was a problem hiding this comment.
Suggestion: When run_id is omitted, this query returns every edge with the field path across all runs. provenance() and the CLI call this method without a run ID, so repeated field paths in separate runs are merged into one chain and can attribute another run's source to the requested value. Require or derive the run context before querying, or otherwise isolate results by run. [api mismatch]
Severity Level: Major ⚠️
- ❌ CLI provenance output can combine separate runs.
- ⚠️ Field-level audit results can show wrong sources.
- ⚠️ Repeated field paths across runs are not isolated.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** storage/postgres.py
**Line:** 569:575
**Comment:**
*Api Mismatch: When `run_id` is omitted, this query returns every edge with the field path across all runs. `provenance()` and the CLI call this method without a run ID, so repeated field paths in separate runs are merged into one chain and can attribute another run's source to the requested value. Require or derive the run context before querying, or otherwise isolate results by run.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| FROM provenance_edges p | ||
| INNER JOIN prov_cte c ON p.field_path = c.source_path | ||
| WHERE c.grade = 'exact' AND c.source_path IS NOT NULL AND c.depth < 50 |
There was a problem hiding this comment.
Suggestion: The recursive step does not restrict p.run_id to the requested run. Even when query_provenance_chain is called with a run ID, an exact edge can follow its source_path into an identically named field from another run, returning cross-run provenance and potentially exposing unrelated data. Add the same run constraint to the recursive join or recursive WHERE clause. [security]
Severity Level: Major ⚠️
- ❌ SQL provenance chains can contain unrelated-run edges.
- ⚠️ Audits receive incorrect cross-run attribution.
- ⚠️ Shared field names make this realistic across repeated runs.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** storage/postgres.py
**Line:** 615:617
**Comment:**
*Security: The recursive step does not restrict `p.run_id` to the requested run. Even when `query_provenance_chain` is called with a run ID, an exact edge can follow its `source_path` into an identically named field from another run, returning cross-run provenance and potentially exposing unrelated data. Add the same run constraint to the recursive join or recursive `WHERE` clause.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
…s, and recursive traversal
| if snapshot is not None: | ||
| if verify: | ||
| verify_snapshot(snapshot, log=log) | ||
| start_state = AgentState.from_json(snapshot.state) |
There was a problem hiding this comment.
Suggestion: Snapshot state is trusted whenever verify remains false, so a corrupted or stale snapshot can silently replace the replayed prefix and produce incorrect reconstruction results. Since the CLI and why call reconstruct without verification, either verify snapshots by default or validate the snapshot hash before using its state. [possible bug]
Severity Level: Major ⚠️
- ❌ Snapshot-enabled reconstruction can return incorrect agent state.
- ⚠️ Later event replay cannot repair skipped snapshot prefixes.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** core/reducer.py
**Line:** 254:257
**Comment:**
*Possible Bug: Snapshot state is trusted whenever `verify` remains false, so a corrupted or stale snapshot can silently replace the replayed prefix and produce incorrect reconstruction results. Since the CLI and `why` call `reconstruct` without verification, either verify snapshots by default or validate the snapshot hash before using its state.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
CodeAnt Nitpicks1 code suggestion1. Ordering a SQL slice causes one database lookup per event.Performance · |
|
@CodeAnt-AI can you review again? |
|
Question: can you review again? Answer: Review findings[High] Recursive PostgreSQL provenance query can cross run boundariesIn FROM provenance_edges p
INNER JOIN prov_cte c ON p.field_path = c.source_path
WHERE c.grade = 'exact'If two runs contain the same For example: Querying Add a run constraint to the recursive join/filter: INNER JOIN prov_cte c
ON p.field_path = c.source_path
AND p.run_id = c.run_id
WHERE c.grade = 'exact'
AND c.source_path IS NOT NULL
AND c.depth < 50This should also be covered by a PostgreSQL test with identical field paths in two different runs. [Medium] Provenance edges may be recorded for events that were not persistedIn if field_sources is not None:
...
record_tool_result_provenance(
result_event,
sources,
store=log,
transform=transform or "tool_call",
)Unlike resource registration, this is not guarded by With Consider recording provenance only when [Low]
|
1 participant
CodeAnt-AI Description
Add historical state reconstruction and trustworthy causal tracing for agent runs
What Changed
whyevidence for decisions, including declared cross-agent inputs and payload mismatchesImpact
✅ Verifiable historical agent state✅ Faster replay after snapshot checkpoints✅ Clearer decision and data-origin tracing💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.