Skip to content

fix(oracle): add project_id to conversation_items and resolve PR #576 feedback - #591

Merged
key4ng merged 2 commits into
mainfrom
keyang/flyway-schema-change
Mar 3, 2026
Merged

key4ng merged 2 commits into
mainfrom
keyang/flyway-schema-change

Conversation

@key4ng

@key4ng key4ng commented Mar 3, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

The Flyway-managed CONVERSATION_ITEMS table lacks a GENERATIVE_AI_PROJECT_ID column needed for access control(thanks @zhaowenzi for pointing it) , and several review comments from PR #576 were acknowledged but not committed.

Solution

Add the GENERATIVE_AI_PROJECT_ID column (with index) to the CONVERSATION_ITEMS DDL, and apply the three outstanding review fixes from #576.

Changes

Summary by CodeRabbit

  • Database Schema

    • Added new column to conversation items for tracking Generative AI project identifiers.
    • Created index to optimize query performance on the new project identifier field.
  • Infrastructure Improvements

    • Enhanced configuration file path resolution and environment variable management.
    • Optimized configuration building logic.

key4ng added 2 commits March 3, 2026 10:53
…ION_ITEMS table

Signed-off-by: Keyang Ru <rukeyang@gmail.com>
…ckend configuration logic

- Changed the schema config path to be dynamically resolved in gateway.py.
- Refactored the backend configuration logic in main.rs to use a match statement for better readability.
- Updated the ci_agentic_svc_deps.sh script to streamline environment variable exports.

Signed-off-by: Keyang Ru <rukeyang@gmail.com>
@coderabbitai

coderabbitai Bot commented Mar 3, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR consolidates and improves code organization across multiple components: dynamic path resolution for Oracle Flyway schema configuration, refactored configuration building in the Rust gateway service via match consolidation, streamlined CI environment variable operations, and a new database column with index for project ID tracking in conversations.

Changes

Cohort / File(s) Summary
Path Resolution
e2e_test/infra/gateway.py
Replaced hard-coded relative path with dynamic absolute path computation using Path(__file__).resolve() for Oracle Flyway schema-config.yaml file reference.
Configuration Refactoring
model_gateway/src/main.rs
Consolidated three separate conditional blocks for oracle/postgres/redis configuration into a single match expression returning a tuple, simplifying control flow without altering external behavior.
CI Script Consolidation
scripts/ci_agentic_svc_deps.sh
Grouped three separate environment variable echo operations into a single block appended to GITHUB_ENV, maintaining variable values while improving script efficiency.
Database Schema
scripts/oracle_flyway/sql/V2__Create_v2_conversations_and_alter_responses.sql
Added new column GENERATIVE_AI_PROJECT_ID VARCHAR2(255) to CONVERSATION_ITEMS table and created corresponding index IDX_CONV_ITEMS_PROJECT_ID.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • smg#576: Modifies the same e2e_test/infra/gateway.py file; this PR improves path handling for the Oracle Flyway schema-config introduced in that PR.

Suggested labels

ci

Suggested reviewers

  • CatherineSue
  • XinyueZhang369
  • slin1237

Poem

🐰 A path once hard-coded, now flows free,
Configuration blocks merge in harmony,
Scripts grow lean, the database expands,
With project IDs tracked by our swift hands,
Cleaner code hops forward, hooray! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding project_id to conversation_items and addressing feedback from PR #576.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch keyang/flyway-schema-change

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added tests Test changes model-gateway Model gateway crate changes labels Mar 3, 2026
@key4ng
key4ng marked this pull request as ready for review March 3, 2026 19:05
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/ci_agentic_svc_deps.sh`:
- Around line 211-215: In cmd_create_oracle_user, replace the three separate
echo lines that each redirect to "$GITHUB_ENV" with a single grouped block
(braced { ... } >> "$GITHUB_ENV") so all ATP_FLYWAY_USER, ATP_FLYWAY_PASSWORD
and ATP_FLYWAY_DSN are written with one redirection; locate the echo calls
inside the cmd_create_oracle_user function and convert them to the grouped form
to match the pattern used elsewhere and avoid repeated file opens.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 770237d and d65d7b4.

📒 Files selected for processing (4)
  • e2e_test/infra/gateway.py
  • model_gateway/src/main.rs
  • scripts/ci_agentic_svc_deps.sh
  • scripts/oracle_flyway/sql/V2__Create_v2_conversations_and_alter_responses.sql

Comment thread scripts/ci_agentic_svc_deps.sh
@key4ng
key4ng merged commit 72bedf1 into main Mar 3, 2026
25 checks passed
@key4ng
key4ng deleted the keyang/flyway-schema-change branch March 3, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model-gateway Model gateway crate changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant