From 47a3f218daa1ea377b876b5614021ff2e7a61467 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 16:14:52 -0400 Subject: [PATCH] fix(artifacts): give every generated file format the same row resolution The CSV work left three gaps in the sibling formats and one loose end. JSON and XML were built only by parsing a payload out of the assistant reply, so they could never use authorized action rows and never reached back to rows an earlier turn already gathered. "make that a json" failed exactly the way "create a csv" used to. build_structured_artifact_rows_payload() now serializes authorized rows, with the same earlier-turn reach-back and the same passthrough contract, whenever the reply carries no payload of its own. The schema clarification gate lived inside the CSV branch, so a turn that asked a clarifying question could still publish a DOCX or PDF. It now runs before the format branch and covers every generated file format. DOCX and PDF guidance never forbade claiming that files cannot be created, which is the exact sentence that produced the truncated CSV. It now states the publication contract the other formats already state. The DOCX and PDF passthrough reason code compared a single row-source literal, so it was dropped once rows could come from an earlier turn. It now uses the named row-source set. Version: 0.260.010 --- application/single_app/config.py | 2 +- .../functions_generated_file_exports.py | 58 +++- application/single_app/route_backend_chats.py | 55 +++- .../test_assistant_table_csv_artifact.py | 9 +- ...st_generated_structured_artifact_parity.py | 261 ++++++++++++++++++ 5 files changed, 365 insertions(+), 20 deletions(-) create mode 100644 functional_tests/test_generated_structured_artifact_parity.py diff --git a/application/single_app/config.py b/application/single_app/config.py index e342eef4a..60b856bf6 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.260.009" +VERSION = "0.260.010" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_generated_file_exports.py b/application/single_app/functions_generated_file_exports.py index 42199ae80..aa1f43e79 100644 --- a/application/single_app/functions_generated_file_exports.py +++ b/application/single_app/functions_generated_file_exports.py @@ -492,10 +492,12 @@ def build_generated_file_output_guidance( ) if output_format in {GENERATED_FILE_FORMAT_DOCX, GENERATED_FILE_FORMAT_PDF}: return ( - f'The user requested a downloadable {output_format.upper()} artifact. Provide a clear final ' - 'response grounded in the available evidence. Structured function results from this turn may ' - 'be included as labeled tables in the generated file; do not invent rows or claim an attachment ' - 'exists before the file-output finalizer publishes it.' + f'The user requested a downloadable {output_format.upper()} artifact. The server renders and ' + 'attaches the file after generation. Provide a clear final response grounded in the available ' + 'evidence. Structured function results from this turn may be included as labeled tables in the ' + 'generated file. Do not claim that you cannot create or attach files, do not tell the user to ' + 'copy or save the content manually, do not invent rows, and do not claim an attachment exists ' + 'before the file-output finalizer publishes it.' ) return '' @@ -533,6 +535,8 @@ def build_generated_file_export( assistant_text = str(assistant_content or '').strip() assistant_rows = extract_assistant_table_entries(assistant_text) + if not assistant_rows and _assistant_reply_requests_clarification(assistant_text): + return None function_rows = extract_authorized_function_result_rows(function_results) row_provenance = ROW_SOURCE_CURRENT_ACTION if not assistant_rows and not function_rows and prior_function_results_loader is not None: @@ -547,8 +551,6 @@ def build_generated_file_export( ) if function_rows else {'allowed': False, 'reason_code': 'source_result_incomplete'} if output_format == GENERATED_FILE_FORMAT_CSV: - if not assistant_rows and _assistant_reply_requests_clarification(assistant_text): - return None use_function_rows = bool(function_rows) and bool(function_passthrough.get('allowed')) and ( not assistant_rows or _assistant_rows_sample_function_rows(assistant_rows, function_rows) @@ -584,8 +586,50 @@ def build_generated_file_export( row_source=row_source, assistant_content=assistant_text, title=title, - passthrough_reason_code=(function_passthrough.get('reason_code') if row_source == 'structured function result' else None), + passthrough_reason_code=( + function_passthrough.get('reason_code') if row_source in FUNCTION_RESULT_ROW_SOURCES else None + ), + ) + + +def build_structured_artifact_rows_payload( + user_question: str, + output_format: str, + function_results: Optional[List[Dict[str, Any]]] = None, + prior_function_results_loader: Optional[Callable[[], Optional[List[Dict[str, Any]]]]] = None, +) -> Optional[Dict[str, Any]]: + """Serialize authorized action rows when a JSON or XML reply did not carry the payload.""" + normalized_output_format = str(output_format or '').strip().lower() + if normalized_output_format not in {'json', 'xml'}: + return None + + function_rows = extract_authorized_function_result_rows(function_results) + row_provenance = ROW_SOURCE_CURRENT_ACTION + if not function_rows and prior_function_results_loader is not None: + function_rows = extract_authorized_function_result_rows(prior_function_results_loader()) + if function_rows: + row_provenance = ROW_SOURCE_EARLIER_ACTION + if not function_rows: + return None + + function_passthrough = evaluate_generated_file_passthrough_eligibility( + user_question, + rows=function_rows, ) + if not function_passthrough.get('allowed'): + return None + + if normalized_output_format == 'json': + file_content = serialize_generated_json(function_rows) + else: + file_content = serialize_generated_xml(function_rows, root_name='GeneratedRows') + return { + 'file_content': file_content, + 'rows': function_rows, + 'row_count': len(function_rows), + 'row_source': row_provenance, + 'passthrough_reason_code': function_passthrough.get('reason_code'), + } def extract_authorized_function_result_rows(function_results: Optional[List[Dict[str, Any]]]) -> List[Dict[str, Any]]: diff --git a/application/single_app/route_backend_chats.py b/application/single_app/route_backend_chats.py index 996343a5a..b0ac2418a 100644 --- a/application/single_app/route_backend_chats.py +++ b/application/single_app/route_backend_chats.py @@ -150,6 +150,7 @@ build_generated_file_artifact_metadata, build_generated_file_export, build_generated_file_output_guidance, + build_structured_artifact_rows_payload, evaluate_generated_file_passthrough_eligibility, estimate_function_result_row_count, get_generated_file_export_content, @@ -2633,11 +2634,26 @@ def _build_streaming_assistant_file_status(output_format): return f'Generating the {normalized_output_format} file. It will appear here when ready.' +def _build_structured_artifact_rows_payload(user_question, output_format, conversation_id, function_results): + """Fall back to authorized action rows when a JSON/XML reply carried no payload.""" + return build_structured_artifact_rows_payload( + user_question, + output_format, + function_results=function_results, + prior_function_results_loader=lambda: _load_prior_turn_function_results( + get_current_user_id(), + conversation_id, + settings=get_settings(), + ), + ) + + def maybe_create_assistant_file_generated_output( user_question, assistant_content, conversation_id, existing_outputs=None, + function_results=None, ): """Save assistant-generated JSON/XML content as a downloadable chat artifact.""" output_format = get_tabular_generated_output_format(user_question) @@ -2650,20 +2666,40 @@ def maybe_create_assistant_file_generated_output( preview_items = [] preview_lines = [] + row_payload = None if output_format == 'json': json_payload = normalize_json_artifact_payload(assistant_content) if json_payload is None: - return None - file_content = serialize_generated_json(json_payload) - if isinstance(json_payload, list): - preview_items = json_payload[:3] - elif isinstance(json_payload, dict): - preview_items = [json_payload] + row_payload = _build_structured_artifact_rows_payload( + user_question, + output_format, + conversation_id, + function_results, + ) + if row_payload is None: + return None + file_content = row_payload['file_content'] + preview_items = row_payload['rows'][:3] + else: + file_content = serialize_generated_json(json_payload) + if isinstance(json_payload, list): + preview_items = json_payload[:3] + elif isinstance(json_payload, dict): + preview_items = [json_payload] else: xml_payload = normalize_xml_artifact_payload(assistant_content) if not xml_payload: - return None - file_content = xml_payload + row_payload = _build_structured_artifact_rows_payload( + user_question, + output_format, + conversation_id, + function_results, + ) + if row_payload is None: + return None + file_content = row_payload['file_content'] + else: + file_content = xml_payload preview_lines = _build_assistant_file_preview_lines(file_content) generated_file_name = _build_assistant_file_export_name(output_format) @@ -15529,6 +15565,7 @@ def execute_document_action_chat_request( assistant_content=document_action_reply_content, conversation_id=conversation_id, existing_outputs=document_generated_analysis_artifacts + document_generated_tabular_outputs, + function_results=execution_result.get('agent_citations') or [], ) if assistant_file_generated_output: document_generated_analysis_artifacts.append(assistant_file_generated_output) @@ -19969,6 +20006,7 @@ def gpt_error(e): assistant_content=ai_message, conversation_id=conversation_id, existing_outputs=generated_analysis_artifacts_list + generated_tabular_outputs_list, + function_results=agent_citations_list, ) if assistant_file_generated_output: generated_analysis_artifacts_list.append(assistant_file_generated_output) @@ -23917,6 +23955,7 @@ def finalize_cancelled_agent_stream_response(): assistant_content=accumulated_content, conversation_id=conversation_id, existing_outputs=generated_analysis_artifacts_list + generated_tabular_outputs_list, + function_results=agent_citations_list, ) if assistant_file_generated_output: generated_analysis_artifacts_list.append(assistant_file_generated_output) diff --git a/functional_tests/test_assistant_table_csv_artifact.py b/functional_tests/test_assistant_table_csv_artifact.py index 256305ccf..295758068 100644 --- a/functional_tests/test_assistant_table_csv_artifact.py +++ b/functional_tests/test_assistant_table_csv_artifact.py @@ -1154,12 +1154,13 @@ def test_chat_route_wires_assistant_table_artifacts(): 'Expected workflow model and agent execution to apply the same file-output guidance.', ) assert_true( - chat_route_content.count('function_results=agent_citations_list') == 2, - 'Expected normal and streaming Chat to pass current-turn action results to generated-file exports.', + chat_route_content.count('function_results=agent_citations_list') == 4, + 'Expected normal and streaming Chat to pass current-turn action results to both the ' + 'generated-file export and the assistant JSON/XML export.', ) assert_true( - 'function_results=execution_result.get(\'agent_citations\') or []' in chat_route_content, - 'Expected document actions to pass current-turn action results to generated-file exports.', + chat_route_content.count('function_results=execution_result.get(\'agent_citations\') or []') == 2, + 'Expected document actions to pass current-turn action results to both export paths.', ) assert_true( 'function_results=raw_agent_citations' in workflow_runner_content, diff --git a/functional_tests/test_generated_structured_artifact_parity.py b/functional_tests/test_generated_structured_artifact_parity.py new file mode 100644 index 000000000..5e24d2e82 --- /dev/null +++ b/functional_tests/test_generated_structured_artifact_parity.py @@ -0,0 +1,261 @@ +# test_generated_structured_artifact_parity.py +#!/usr/bin/env python3 +""" +Functional test for generated artifact parity across CSV, DOCX, PDF, JSON, and XML. +Version: 0.260.010 +Implemented in: 0.260.010 + +The CSV work in 0.260.007 through 0.260.009 left three gaps in the sibling +formats and one loose end: + + 1. JSON and XML were built only by parsing the payload out of the assistant + reply, so they could never use authorized action rows and never reached + back to rows an earlier turn already gathered. + 2. The schema clarification gate lived inside the CSV branch, so a turn that + asked a clarifying question could still publish a DOCX or PDF. + 3. DOCX and PDF guidance never forbade claiming that files cannot be created, + which is the exact sentence that produced the truncated CSV. + 4. The DOCX and PDF passthrough reason code compared one row-source literal, + so it was dropped when rows came from an earlier turn. + +This test ensures every generated file format resolves rows the same way and +refuses to publish on a clarification turn. +""" + +import sys +import traceback +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +APP_DIR = ROOT / 'application' / 'single_app' +if str(APP_DIR) not in sys.path: + sys.path.insert(0, str(APP_DIR)) +if str(Path(__file__).resolve().parent) not in sys.path: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +from functions_generated_file_exports import ( # noqa: E402 + build_generated_file_export, + build_generated_file_output_guidance, + build_structured_artifact_rows_payload, +) + +IMPLEMENTED_VERSION = '0.260.010' + +TELEMETRY_ROWS = [ + {'id': f'R-{index}', 'voltage': 27 + index % 5, 'state': 'CRITICAL'} + for index in range(30) +] +TELEMETRY_FUNCTION_RESULTS = [{ + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'row_count': len(TELEMETRY_ROWS), 'rows': TELEMETRY_ROWS}, +}] +CLARIFICATION_REPLY = ( + 'Should each row represent one telemetry sample, and which columns should it include?' +) + + +def assert_true(condition, message): + if not condition: + raise AssertionError(message) + + +def test_json_and_xml_use_authorized_action_rows(): + """A JSON or XML request must serialize action rows when the reply carries no payload.""" + print('Testing JSON/XML row fallback...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + json_payload = build_structured_artifact_rows_payload( + 'create a json of that data', + 'json', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + xml_payload = build_structured_artifact_rows_payload( + 'create an xml of that data', + 'xml', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(json_payload is not None, 'Expected a JSON payload from authorized action rows.') + assert_true( + json_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected every authorized row in the JSON artifact.', + ) + assert_true(json_payload['file_content'].lstrip().startswith('['), 'Expected a JSON array payload.') + assert_true(xml_payload is not None, 'Expected an XML payload from authorized action rows.') + assert_true( + '' in xml_payload['file_content'], + 'Expected the shared generated-rows XML root.', + ) + + +def test_json_and_xml_reach_back_to_earlier_turns(): + """JSON and XML must reuse rows an earlier turn gathered, the same way CSV does.""" + print('Testing JSON/XML earlier-turn reach-back...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + json_payload = build_structured_artifact_rows_payload( + 'create a json', + 'json', + function_results=[], + prior_function_results_loader=lambda: TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(json_payload is not None, 'Expected the follow-up JSON request to reuse earlier rows.') + assert_true( + json_payload['row_source'] == 'earlier action result', + 'Expected reused rows to declare their earlier-turn provenance.', + ) + assert_true( + json_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected the full earlier result set in the JSON artifact.', + ) + + +def test_row_fallback_is_limited_to_structured_formats(): + """CSV, DOCX, and PDF keep their own renderers; the fallback must not claim them.""" + print('Testing structured row fallback scope...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for output_format in ('csv', 'docx', 'pdf', '', None): + assert_true( + build_structured_artifact_rows_payload( + 'create a file', + output_format, + function_results=TELEMETRY_FUNCTION_RESULTS, + ) is None, + f'Expected {output_format!r} to be handled by its own renderer.', + ) + + +def test_row_fallback_requires_a_passthrough_contract(): + """Rows must still satisfy the passthrough contract before they become an artifact.""" + print('Testing structured row fallback eligibility...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + assert_true( + build_structured_artifact_rows_payload( + 'analyze the telemetry and summarize each sample', + 'json', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) is None, + 'Expected a derived-output request to refuse raw row passthrough.', + ) + assert_true( + build_structured_artifact_rows_payload('create a json', 'json', function_results=[]) is None, + 'Expected no artifact when no rows are available.', + ) + + +def test_clarification_turn_publishes_no_artifact_in_any_format(): + """A clarifying question must not publish a CSV, DOCX, or PDF.""" + print('Testing clarification gate across formats...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for question in ('create a csv', 'create a word document', 'create a pdf'): + assert_true( + build_generated_file_export( + question, + CLARIFICATION_REPLY, + function_results=TELEMETRY_FUNCTION_RESULTS, + ) is None, + f'Expected no artifact for a clarification turn on: {question}', + ) + + +def test_docx_and_pdf_still_publish_a_normal_answer(): + """The clarification gate must not suppress ordinary document requests.""" + print('Testing DOCX/PDF normal publication...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for question in ('create a word document', 'create a pdf'): + export_payload = build_generated_file_export( + question, + 'Here is the summary of the retrieved samples.', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + assert_true(export_payload is not None, f'Expected an artifact for: {question}') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + f'Expected the authorized rows to be embedded for: {question}', + ) + + +def test_docx_and_pdf_keep_the_reason_code_for_earlier_rows(): + """Provenance must not drop the passthrough reason code when rows come from earlier.""" + print('Testing DOCX/PDF passthrough reason code...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + 'create a word document', + '', + function_results=[], + prior_function_results_loader=lambda: TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected a DOCX artifact from earlier rows.') + assert_true( + export_payload['row_source'] == 'earlier action result', + 'Expected earlier-turn provenance on the DOCX artifact.', + ) + assert_true( + export_payload.get('passthrough_reason_code') == 'explicit_format_conversion', + 'Expected the passthrough reason code to survive earlier-turn provenance.', + ) + + +def test_docx_and_pdf_guidance_states_the_publication_contract(): + """DOCX and PDF must not be left able to claim they cannot create files.""" + print('Testing DOCX/PDF guidance parity...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for question in ('create a word document', 'create a pdf'): + guidance = build_generated_file_output_guidance(question) + assert_true( + 'attaches the file after generation' in guidance, + f'Expected the publication contract in guidance for: {question}', + ) + assert_true( + 'cannot create or attach files' in guidance, + f'Expected the no-refusal rule in guidance for: {question}', + ) + assert_true( + 'do not invent rows' in guidance, + f'Expected the existing no-invented-rows rule to survive for: {question}', + ) + + +def run_tests() -> bool: + tests = [ + test_json_and_xml_use_authorized_action_rows, + test_json_and_xml_reach_back_to_earlier_turns, + test_row_fallback_is_limited_to_structured_formats, + test_row_fallback_requires_a_passthrough_contract, + test_clarification_turn_publishes_no_artifact_in_any_format, + test_docx_and_pdf_still_publish_a_normal_answer, + test_docx_and_pdf_keep_the_reason_code_for_earlier_rows, + test_docx_and_pdf_guidance_states_the_publication_contract, + ] + + results = [] + for test in tests: + print(f'\nRunning {test.__name__}...') + try: + test() + print(f'{test.__name__} passed') + results.append(True) + except Exception as exc: + print(f'{test.__name__} failed: {exc}') + traceback.print_exc() + results.append(False) + + passed = sum(1 for result in results if result) + print(f'\nResults: {passed}/{len(tests)} tests passed') + return all(results) + + +if __name__ == '__main__': + sys.exit(0 if run_tests() else 1)