Conversation
|
@claude review this pr according to description. |
|
Claude encountered an error —— View job - Event type: PR general comment triggered by `@claude` on an open PR. - Request type: code review only; no implementation requested. - Key context: PR #11492 (`oracle-separated-bind` -> `main`) updates `agent/src/flow_generator/protocol_logs/sql/oracle.rs` and keeps `agent/crates/enterprise-utils/src/lib.rs` aligned. - Review focus: whether request/bind state is cached correctly across frames so custom SQL extraction can resolve bind values when SQL and binds are separated. - Plan: check repo instructions, inspect the diff against `origin/main`, read the touched files for surrounding context, then post review findings. I can inspect code and diffs locally; I do not currently plan to run a full test suite unless needed for a specific concern. Tasks
|
5f360db to
dd68cbc
Compare
|
@claude review this pr according to description |
|
Claude finished @rvql's task in 17m 33s —— View job - Event type: PR general comment on an open PR. - Request type: code review only. - Scope reviewed: `agent/src/flow_generator/protocol_logs/sql/oracle.rs` and the matching `enterprise-utils` stub updates. - Focus: whether caching request info across frames correctly supports custom SQL extraction when Oracle SQL text and bind values arrive separately. Tasks
ReviewI found 2 correctness issues.
SummaryThe overall direction makes sense, but in the current form it changes request timing semantics and can drop some valid Oracle requests entirely under custom-policy configurations. |
Cache Oracle request info across frames so custom SQL extraction can resolve bind values when SQL and binds arrive separately. Keep the local enterprise-utils stubs aligned with the parser changes.
1991870 to
07a9f95
Compare
|
@claude review it |
- Event type: PR general comment on an open PR.
- Request type: code review only (`@claude review it`), so I should inspect the PR changes and report review feedback without implementing changes.
- Key context: PR #11492 (`oracle-separated-bind` -> `main`) updates `agent/src/flow_generator/protocol_logs/sql/oracle.rs` plus the matching `agent/crates/enterprise-utils/src/lib.rs` stub.
- Review focus: whether Oracle request state is cached correctly across frames so custom SQL extraction can resolve binds when SQL text and bind values arrive separately.
- Plan: read repo instructions from `CLAUDE.md`, inspect `git diff origin/main...HEAD`, read the touched files in context, assess correctness and regressions, then update this comment with findings. I may run targeted git inspection commands; I do not plan to modify code unless explicitly asked.
Tasks
|
|
Claude finished @rvql's task in 27m 35s —— View job - Event type: PR general comment on an open PR. - Request type: code review only; the trigger comment says `@claude review it`. - Key context: review PR #11492 (`oracle-separated-bind` -> `main`) against the description about caching Oracle request info across frames so custom SQL extraction can resolve separated bind values. - Files in scope: `agent/src/flow_generator/protocol_logs/sql/oracle.rs` and `agent/crates/enterprise-utils/src/lib.rs`. - Main tasks: read repository instructions, inspect the diff against `origin/main`, validate behavior for split SQL/bind frames, and post concise review feedback with file references. - Plan completed: read the touched code and surrounding parser/perf/session code, then update this comment with findings. I did not implement changes. Tasks
ReviewI found 1 correctness issue.
SummaryThe cross-frame bind merge logic looks directionally right, and the placeholder detection is much safer than before because it now tokenizes only the supported upsert regions. The remaining problem is lifecycle-related: once a request is hidden behind |

Cache Oracle request info across frames so custom SQL extraction can resolve bind values when SQL and binds arrive separately. Keep the local enterprise-utils stubs aligned with the parser changes.
This PR is for: