Update C chat samples with using ChatHistory class #3045

zhaohb · 2025-11-19T05:35:23Z

No description provided.

src/c/include/openvino/genai/c/chat_history.h

src/c/src/chat_history.cpp

src/c/include/openvino/genai/c/chat_history.h

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-05T01:58:32Z

samples/c/text_generation/chat_sample_c.c

+        snprintf(message_json, sizeof(message_json), 
+                 "{\"role\": \"user\", \"content\": \"%s\"}", escaped_prompt);


The snprintf call doesn't check for truncation. If the formatted string exceeds MAX_JSON_LENGTH, snprintf will truncate it and return a value >= MAX_JSON_LENGTH. This could result in malformed JSON being passed to ov_genai_json_container_create_from_json_string, leading to JSON parsing errors. Consider checking the return value of snprintf to ensure the formatted string fits within the buffer.

src/c/include/openvino/genai/c/chat_history.h

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-05T04:36:14Z

samples/c/text_generation/chat_sample_c.c

+static void json_escape_string(const char* input, char* output, size_t output_size) {
+    size_t i = 0;
+    size_t j = 0;
+    while (input[i] != '\0' && j < output_size - 1) {
+        switch (input[i]) {
+            case '"':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = '"';
+                }
+                break;
+            case '\\':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = '\\';
+                }
+                break;
+            case '\n':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = 'n';
+                }
+                break;
+            case '\r':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = 'r';
+                }
+                break;
+            case '\t':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = 't';
+                }
+                break;
+            default:
+                output[j++] = input[i];
+                break;
+        }
+        i++;
+    }
+    output[j] = '\0';
+}


The JSON escape function has incomplete escape handling. It's missing essential JSON escape sequences like '\b' (backspace) and '\f' (form feed). Additionally, control characters (0x00-0x1F) should be escaped as "\uXXXX" according to JSON specification. Consider using a more complete JSON escaping implementation or a JSON library.

Copilot · 2025-12-05T04:36:15Z

samples/c/text_generation/chat_sample_c.c

+        snprintf(message_json, sizeof(message_json), 
+                 "{\"role\": \"user\", \"content\": \"%s\"}", escaped_prompt);


Potential buffer overflow: snprintf is called with sizeof(message_json) (4096 bytes) but writes escaped_prompt which can be up to (MAX_PROMPT_LENGTH - 1) * 2 + 1 = 2047 bytes, plus the JSON template string {"role": "user", "content": ""} (32 bytes). If the user enters close to MAX_PROMPT_LENGTH characters with many escapable characters, the result could exceed the 4096 byte buffer. The buffer size calculation should account for the worst-case scenario of the escaped string plus JSON formatting overhead.

Copilot · 2025-12-05T04:36:15Z

samples/c/text_generation/chat_sample_c.c

-                                                    NULL));  // Only the streamer functionality is used here.
+
+        // Skip empty lines
+        if (strlen(prompt) == 0) {


The loop continues when an empty line is encountered, but this will cause "question:" to not be printed after skipping. This means multiple empty lines will result in no prompt being shown. Consider printing the prompt after the continue statement, or restructure the logic to handle empty lines more gracefully.

Suggested change

if (strlen(prompt) == 0) {

if (strlen(prompt) == 0) {

printf("question:\n");

Copilot · 2025-12-05T04:36:15Z

src/c/include/openvino/genai/c/chat_history.h

+ * @param history A pointer to the newly created ov_genai_chat_history.
+ * @param messages A JsonContainer containing an array of message objects.
+ * @return ov_genai_chat_history_status_e A status code, return OK(0) if successful.
+ */
+OPENVINO_GENAI_C_EXPORTS ov_genai_chat_history_status_e ov_genai_chat_history_create_from_json_container(
+    ov_genai_chat_history** history,
+    const ov_genai_json_container* messages


[nitpick] Inconsistent parameter ordering with similar functions. This function has parameters ordered as (output, input), while ov_genai_json_container_create_from_json_string uses (input, output) ordering. For consistency across the API, consider reordering to match the pattern where input parameters come before output parameters.

Suggested change

* @param history A pointer to the newly created ov_genai_chat_history.

* @param messages A JsonContainer containing an array of message objects.

* @return ov_genai_chat_history_status_e A status code, return OK(0) if successful.

*/

OPENVINO_GENAI_C_EXPORTS ov_genai_chat_history_status_e ov_genai_chat_history_create_from_json_container(

ov_genai_chat_history** history,

const ov_genai_json_container* messages

* @param messages A JsonContainer containing an array of message objects.

* @param history A pointer to the newly created ov_genai_chat_history.

* @return ov_genai_chat_history_status_e A status code, return OK(0) if successful.

*/

OPENVINO_GENAI_C_EXPORTS ov_genai_chat_history_status_e ov_genai_chat_history_create_from_json_container(

const ov_genai_json_container* messages,

ov_genai_chat_history** history

Please address Copilot's proposal - align order of input/output params

yatarkan

Please also check and apply Copilot's comments or explain if some of them not applicable

yatarkan · 2025-12-05T10:59:07Z

src/c/include/openvino/genai/c/chat_history.h

+ * @param history A pointer to the newly created ov_genai_chat_history.
+ * @param messages A JsonContainer containing an array of message objects.
+ * @return ov_genai_chat_history_status_e A status code, return OK(0) if successful.
+ */
+OPENVINO_GENAI_C_EXPORTS ov_genai_chat_history_status_e ov_genai_chat_history_create_from_json_container(
+    ov_genai_chat_history** history,
+    const ov_genai_json_container* messages


Please address Copilot's proposal - align order of input/output params

yatarkan · 2025-12-05T12:30:17Z

src/c/src/chat_history.cpp

+        return OV_GENAI_CHAT_HISTORY_INVALID_PARAM;
+    }
+    try {
+        history->object->push_back(*(message->object));


Let's check that message is json container object

yatarkan · 2025-12-05T12:42:05Z

src/c/src/chat_history.cpp

+        if (index >= history->object->size()) {
+            return OV_GENAI_CHAT_HISTORY_OUT_OF_BOUNDS;
+        }
+        ov::genai::JsonContainer message_ref = (*history->object)[index];


Is message_ref actually a reference? If not let's rename accordingly. The same for methods below.

yatarkan · 2025-12-05T12:48:33Z

src/c/include/openvino/genai/c/json_container.h

+    const ov_genai_json_container* container,
+    ov_genai_json_container** copy_container);


Let's name it a bit more explicitly for clarification

Suggested change

const ov_genai_json_container* container,

ov_genai_json_container** copy_container);

const ov_genai_json_container* source,

ov_genai_json_container** target);

Update C chat samples with using ChatHistory class

5a41c21

Copilot AI review requested due to automatic review settings November 19, 2025 05:35

github-actions bot added category: LLM samples GenAI LLM samples category: C API labels Nov 19, 2025

Copilot started reviewing on behalf of zhaohb November 19, 2025 05:36 View session

Copilot finished reviewing on behalf of zhaohb November 19, 2025 05:37

This comment was marked as duplicate.

Sign in to view

update

c28ec23

Wovchena requested a review from yatarkan November 19, 2025 07:57

Merge branch 'master' into chat_history_c_api

f114d11

Copilot AI review requested due to automatic review settings November 19, 2025 10:16

Copilot started reviewing on behalf of zhaohb November 19, 2025 10:17 View session

Copilot finished reviewing on behalf of zhaohb November 19, 2025 10:19

This comment was marked as duplicate.

Sign in to view

yatarkan requested changes Nov 19, 2025

View reviewed changes

zhaohb added 2 commits December 1, 2025 10:23

Merge branch 'master' into chat_history_c_api

04089cc

use JsonContainer

bbfceac

Copilot AI review requested due to automatic review settings December 3, 2025 08:40

Copilot started reviewing on behalf of zhaohb December 3, 2025 08:41 View session

Copilot finished reviewing on behalf of zhaohb December 3, 2025 08:43

Simplify the logic for catching errors

0dc1226

This comment was marked as duplicate.

Sign in to view

zhaohb requested a review from yatarkan December 3, 2025 08:50

Apply suggestion

ce474f7

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings December 3, 2025 09:02

Copilot started reviewing on behalf of zhaohb December 3, 2025 09:03 View session

zhaohb and others added 2 commits December 3, 2025 17:03

Apply suggestion

6948a30

Co-authored-by: Copilot <[email protected]>

Apply suggestion

584652c

Co-authored-by: Copilot <[email protected]>

Copilot finished reviewing on behalf of zhaohb December 3, 2025 09:06

Apply suggestion

74fbb66

Co-authored-by: Copilot <[email protected]>

This comment was marked as duplicate.

Sign in to view

Merge branch 'master' into chat_history_c_api

38db49d

zhaohb requested a review from Copilot December 4, 2025 01:29

Copilot started reviewing on behalf of zhaohb December 4, 2025 01:30 View session

Copilot finished reviewing on behalf of zhaohb December 4, 2025 01:33

Copilot AI reviewed Dec 4, 2025

View reviewed changes

zhaohb and others added 2 commits December 5, 2025 09:44

Merge branch 'master' into chat_history_c_api

9852ddc

update

ef4eb1d

Copilot AI review requested due to automatic review settings December 5, 2025 01:54

Copilot started reviewing on behalf of zhaohb December 5, 2025 01:54 View session

Copilot finished reviewing on behalf of zhaohb December 5, 2025 01:57

Copilot AI reviewed Dec 5, 2025

View reviewed changes

Update src/c/include/openvino/genai/c/chat_history.h

49ab11b

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings December 5, 2025 04:31

Copilot started reviewing on behalf of zhaohb December 5, 2025 04:32 View session

Copilot finished reviewing on behalf of zhaohb December 5, 2025 04:34

Copilot AI reviewed Dec 5, 2025

View reviewed changes

yatarkan requested changes Dec 5, 2025

View reviewed changes

		snprintf(message_json, sizeof(message_json),
		"{\"role\": \"user\", \"content\": \"%s\"}", escaped_prompt);

	if (strlen(prompt) == 0) {
	if (strlen(prompt) == 0) {
	printf("question:\n");

		const ov_genai_json_container* container,
		ov_genai_json_container** copy_container);

Update C chat samples with using ChatHistory class #3045

Are you sure you want to change the base?

Update C chat samples with using ChatHistory class #3045

Conversation

zhaohb commented Nov 19, 2025

Uh oh!

This comment was marked as duplicate.

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

This comment was marked as duplicate.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

yatarkan Dec 5, 2025 • 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!

yatarkan left a comment

Choose a reason for hiding this comment

Uh oh!

yatarkan Dec 5, 2025 • 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!

yatarkan Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

yatarkan Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

yatarkan Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yatarkan Dec 5, 2025 •

edited

Loading

yatarkan Dec 5, 2025 •

edited

Loading