Feedback tests for async and sync profiles by Vishwas-testing · Pull Request #23 · oracle/python-select-ai

Vishwas-testing · 2026-02-25T10:08:58Z

Description

Feedback Select AI tests

Test only transaction

JJsudo7

Hi Vishwas,

Looks good overall.
Review comments/suggestions have been provided. Please check.

Thanks!

JJsudo7 · 2026-03-18T09:51:23Z

+    profile.delete_feedback(prompt_spec=(prompt, action))
+    logger.info("Retrieving prompt after deletion to verify removal")
+    show_prompt = profile.show_prompt(DUMMY_PROMPT)
+    assert response not in show_prompt


showprompt is a good downstream validation for response parameter.
But for feedback_content parameter, we might upstream validation of feedback index table. Is it possible to add?

JJsudo7 · 2026-03-18T09:55:04Z

+
+
+def test_4004(profile):
+    """CASE : Negative feedback test with SHOWSQL ordering by name"""


Could you highlight difference btw test test case 4001 and 4004?

Also, negative coverage for non supported actions like NARRATE, CHAT required in Python?

might not be needed as it is delegating call to dbms_cloud_ai.feedback

JJsudo7 · 2026-03-18T10:02:34Z

+    )
+    feedback_content = "print in descending order of total_points"
+    logger.info(
+        "Expecting DatabaseError when adding negative feedback with prompt=%s action=%s sql_id=%s",


When providing SQL_ID and SQL_TEXT we can have cases -

SQL_ID and SQL_TEXT match - Expected: Feedback addition passes

SQL_ID and SQL_TEXT mismatch - Expected error "ORA-20000: SQL_ID does not match with the SQL text"
Please check if these cases are covered?

Test 4007 is case 2 since error expected?

Both sql_id and sql_text are not supported in the function . This behavour is consistent with dbms_cloud_ai.feedback.

covered in new commit

JJsudo7 · 2026-03-18T10:05:29Z

+    feedback_content = "print in descending order of total_points"
+    logger.info("Adding negative feedback for prompt=%s action=%s", prompt, action)
+
+    profile.add_negative_feedback(


I see another add_negative_feedback() call in this test case with SQL_ID. Would this be needed?

JJsudo7 · 2026-03-18T10:24:08Z

+    assert prompt in show_prompt
+
+    with pytest.raises(oracledb.DatabaseError) as exc_info:
+        profile.add_positive_feedback(


Can we have logging for add_positive_feedback?

JJsudo7 · 2026-03-18T10:30:14Z

+
+
+def test_4013(profile):
+    """CASE : SQL_TEXT DOES NOT MATCH ANY EXISTING FEEDBACK SQL_TEXT"""


Please have similar test case for Negative Feedback.

Txn lauzhao_bug-38415980 introduced negative feedback addition without need of existing SQL_TEXT. Hence, negative feedback addition with non-existing SQL_TEXT should pass.

Please refer txn joejos_rti-32183121 for the additional tests I have added for this.

JJsudo7 · 2026-03-18T10:36:45Z

+    assert response not in show_prompt
+    assert prompt not in show_prompt
+
+


Please check if we have test case for feedback deletion with SQL_ID?

JJsudo7 · 2026-03-18T10:38:58Z

+
+
+async def test_4119(profile):
+    """CASE : SQL_TEXT DOES NOT MATCH ANY EXISTING FEEDBACK SQL_TEXT"""


Similar maybe added for SQL_ID?

JJsudo7 · 2026-03-18T10:44:38Z

+    sql_id = "sql_id_mismatch"
+
+    with pytest.raises(oracledb.DatabaseError) as exc_info:
+        profile.add_positive_feedback(sql_id=sql_id)


Please check if we have positive feedback with valid SQL_ID?

Pushpendra-Garg · 2026-03-23T10:34:18Z

+        feedback_content=feedback_content,
+    )
+    logger.info("Retrieving prompt via show_prompt for dummy input")
+    show_prompt = await profile.show_prompt(dummyPrompt)


Please log show_prompt for LRG integration.

Pushpendra-Garg · 2026-03-23T10:34:50Z

+        prompt_spec=(prompt, action)
+    )
+    logger.info("Retrieving prompt after deletion to verify removal")
+    show_prompt = await profile.show_prompt(dummyPrompt)


Should it be logged ?

Pushpendra-Garg · 2026-03-23T10:36:23Z

+        feedback_content=feedback_content,
+    )
+    logger.info("Retrieving prompt via show_prompt for dummy input")
+    show_prompt = await profile.show_prompt(dummyPrompt)


Please make sure to add additional logging as applicable.

Pushpendra-Garg · 2026-03-23T10:37:56Z

+    logger.info("Deleting feedback for prompt=%s action=%s", prompt, action)
+    await profile.delete_feedback(
+        prompt_spec=(prompt, action)
+    )


Verify if prompt is removed ?

That is being verified using assert statements

Pushpendra-Garg · 2026-03-23T10:39:19Z

+        prompt_spec=(prompt, action),
+        response=response,
+        feedback_content=feedback_content,
+    )


Is there a way to confirm is feedback is added ?

yes added logging for feedback table

Vishwas-testing

Used cursor for tests as acknowledged by abhishek

Feedback tests for async and sync profiles Adding feedback logging address comments reviewed comments comments correction

JJsudo7 · 2026-04-01T05:02:28Z

+    prompt = PROMPT
+    action = Action.SHOWSQL
+    sql_id = SHOWSQL_SQL_ID
+    feedback_content = "print in ascending order of total_points"


In dbms_cloud_ai.feedback, providing negative feedback for existent SQL with just feedback content would yield LLM interaction and pass.

Please check if the disparity in behaviour btw python and SQL expected?

sure i will check and file a bug if needed

Pushpendra-Garg

Changes look good.

oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Feb 25, 2026

Vishwas-testing force-pushed the feedback-tests branch from d347490 to 6ad7297 Compare February 25, 2026 10:26

Vishwas-testing requested review from Pushpendra-Garg and prateeks99 February 25, 2026 16:48

Vishwas-testing force-pushed the feedback-tests branch from ea9738b to cafac1b Compare February 26, 2026 04:56

Pushpendra-Garg reviewed Mar 17, 2026

View reviewed changes

Comment thread tests/feedback/conftest.py Outdated

Vishwas-testing force-pushed the feedback-tests branch from 3d95732 to ffe93a9 Compare March 17, 2026 14:27

Vishwas-testing requested a review from Pushpendra-Garg March 17, 2026 14:28

prateeks99 previously approved these changes Mar 18, 2026

View reviewed changes

JJsudo7 reviewed Mar 18, 2026

View reviewed changes

Pushpendra-Garg reviewed Mar 23, 2026

View reviewed changes

Vishwas-testing dismissed prateeks99’s stale review via 8aa01b2 March 30, 2026 12:35

Vishwas-testing force-pushed the feedback-tests branch from 8aa01b2 to dfe772b Compare March 30, 2026 12:37

Vishwas-testing commented Mar 30, 2026

View reviewed changes

Vishwas-testing requested review from JJsudo7, Pushpendra-Garg and prateeks99 March 30, 2026 12:38

Feedback tests for async and sync profiles

b20850f

Feedback tests for async and sync profiles Adding feedback logging address comments reviewed comments comments correction

Vishwas-testing force-pushed the feedback-tests branch from b9547c8 to b20850f Compare April 1, 2026 04:42

JJsudo7 approved these changes Apr 1, 2026

View reviewed changes

Pushpendra-Garg approved these changes Apr 9, 2026

View reviewed changes

Vishwas-testing merged commit cc524e2 into main Apr 9, 2026
1 of 41 checks passed



		def test_4004(profile):
		"""CASE : Negative feedback test with SHOWSQL ordering by name"""



		def test_4013(profile):
		"""CASE : SQL_TEXT DOES NOT MATCH ANY EXISTING FEEDBACK SQL_TEXT"""

		assert response not in show_prompt
		assert prompt not in show_prompt



		async def test_4119(profile):
		"""CASE : SQL_TEXT DOES NOT MATCH ANY EXISTING FEEDBACK SQL_TEXT"""

Conversation

Vishwas-testing commented Feb 25, 2026

Description

Uh oh!

Uh oh!

JJsudo7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vishwas-testing Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vishwas-testing Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vishwas-testing left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pushpendra-Garg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Vishwas-testing Mar 24, 2026 •

edited

Loading

Vishwas-testing Mar 30, 2026 •

edited

Loading