diff --git a/conftest.py b/conftest.py index 94d508381..2ee2ef734 100644 --- a/conftest.py +++ b/conftest.py @@ -2,23 +2,27 @@ Test configuration for agent tests. """ +import sys +from pathlib import Path + import pytest # Add the agents path agents_path = Path(__file__).parent.parent.parent / "backend" / "v4" / "magentic_agents" sys.path.insert(0, str(agents_path)) + @pytest.fixture def agent_env_vars(): """Common environment variables for agent testing.""" return { "BING_CONNECTION_NAME": "test_bing_connection", "MCP_SERVER_ENDPOINT": "http://test-mcp-server", - "MCP_SERVER_NAME": "test_mcp_server", + "MCP_SERVER_NAME": "test_mcp_server", "MCP_SERVER_DESCRIPTION": "Test MCP server", "TENANT_ID": "test_tenant_id", "CLIENT_ID": "test_client_id", "AZURE_OPENAI_ENDPOINT": "https://test.openai.azure.com/", "AZURE_OPENAI_API_KEY": "test_key", "AZURE_OPENAI_DEPLOYMENT_NAME": "test_deployment" - } \ No newline at end of file + } diff --git a/src/backend/api/router.py b/src/backend/api/router.py index 6af07485a..dbf11afe6 100644 --- a/src/backend/api/router.py +++ b/src/backend/api/router.py @@ -1466,4 +1466,4 @@ async def get_generated_image(blob_name: str): return Response(content=data, media_type="image/png") except Exception as exc: logging.error(f"Error retrieving image '{blob_name}': {exc}") - raise HTTPException(status_code=404, detail="Image not found") \ No newline at end of file + raise HTTPException(status_code=404, detail="Image not found") diff --git a/src/backend/callbacks/response_handlers.py b/src/backend/callbacks/response_handlers.py index 663c9b184..26c964b31 100644 --- a/src/backend/callbacks/response_handlers.py +++ b/src/backend/callbacks/response_handlers.py @@ -13,6 +13,7 @@ AgentToolCall, AgentToolMessage, WebsocketMessageType) from orchestration.connection_config import connection_config +from common.utils.markdown_utils import normalize_markdown_tables logger = logging.getLogger(__name__) @@ -73,6 +74,9 @@ def agent_response_callback( text = clean_citations(text or "") + # Repair collapsed markdown tables before rendering (Bug 47810). + text = normalize_markdown_tables(text) + if not user_id: logger.debug("No user_id provided; skipping websocket send for final message.") return diff --git a/src/backend/common/utils/markdown_utils.py b/src/backend/common/utils/markdown_utils.py new file mode 100644 index 000000000..27e8a7e9b --- /dev/null +++ b/src/backend/common/utils/markdown_utils.py @@ -0,0 +1,70 @@ +"""Shared GFM table repair utilities (Bug 47810).""" + +import re +from typing import List, Optional + +# Matches a GFM delimiter cell: "---", ":--", "--:", ":-:". +_TABLE_DELIM_CELL_RE = re.compile(r"^\s*:?-+:?\s*$") + + +def reflow_collapsed_table_line(line: str) -> Optional[List[str]]: + """Rebuild a GFM table flattened onto one line; return None if not collapsed.""" + if line.count("|") < 4 or "-" not in line: + return None + + first = line.index("|") + prefix = line[:first].rstrip() + raw = line[first:].rstrip() + + tokens = raw.split("|") + if tokens and tokens[0].strip() == "": + tokens = tokens[1:] + if tokens and tokens[-1].strip() == "": + tokens = tokens[:-1] + if not tokens: + return None + + # Whitespace-only tokens mark the boundary between flattened rows. + rows: List[List[str]] = [] + current: List[str] = [] + for tok in tokens: + if tok.strip() == "": + if current: + rows.append(current) + current = [] + else: + current.append(tok) + if current: + rows.append(current) + + # Require a header plus a delimiter row; leaves well-formed tables untouched. + if len(rows) < 2 or not all(_TABLE_DELIM_CELL_RE.match(c) for c in rows[1]): + return None + + n = len(rows[1]) + if n == 0 or len(rows[0]) != n: + return None + + rendered = ["| " + " | ".join(cell.strip() for cell in row) + " |" for row in rows] + + result: List[str] = [] + if prefix: + result.append(prefix) + result.append("") # Blank line so GFM starts a fresh table block. + result.extend(rendered) + return result + + +def normalize_markdown_tables(text: str) -> str: + """Repair collapsed GFM tables; non-table text is returned unchanged.""" + if not text or "|" not in text or "-" not in text: + return text + + out: List[str] = [] + for line in text.split("\n"): + reflowed = reflow_collapsed_table_line(line) + if reflowed is None: + out.append(line) + else: + out.extend(reflowed) + return "\n".join(out) diff --git a/src/backend/orchestration/orchestration_manager.py b/src/backend/orchestration/orchestration_manager.py index d66ba9adb..4c935630f 100644 --- a/src/backend/orchestration/orchestration_manager.py +++ b/src/backend/orchestration/orchestration_manager.py @@ -20,6 +20,8 @@ from common.config.app_config import config from common.database.database_base import DatabaseBase from common.models.messages import TeamConfiguration +from common.utils.markdown_utils import \ + normalize_markdown_tables as _normalize_markdown_tables from models.messages import AgentMessageStreaming, WebsocketMessageType from orchestration.connection_config import (connection_config, orchestration_config) @@ -35,18 +37,18 @@ apply_tool_history_leak_patch() _BARE_IMAGE_URL_RE = re.compile( - r"(? None: # accumulated orchestrator streaming chunks. final_text = final_output_ref[0] or "".join(orchestrator_chunks) + # Repair collapsed markdown tables before rendering (Bug 47810). + final_text = _normalize_markdown_tables(final_text) + final_text = _embed_bare_image_urls(final_text) # Issue 1 diagnostic: confirm the final answer carries a renderable image @@ -852,4 +852,4 @@ async def _process_event_stream( if tool_approvals: result["tool_approvals"] = tool_approvals return result - return None \ No newline at end of file + return None diff --git a/src/tests/backend/orchestration/test_orchestration_manager.py b/src/tests/backend/orchestration/test_orchestration_manager.py index 879e76145..15245883e 100644 --- a/src/tests/backend/orchestration/test_orchestration_manager.py +++ b/src/tests/backend/orchestration/test_orchestration_manager.py @@ -177,6 +177,19 @@ def _make_workflow_mock(run_return=None, executors=None): sys.modules['common.config.app_config'] = Mock(config=mock_config) sys.modules['common.models'] = Mock() +# Register the real markdown_utils so the orchestrator uses genuine table logic, not a Mock (Bug 47810). +import importlib.util as _ilu # noqa: E402 + +_md_path = os.path.join( + os.path.dirname(__file__), "..", "..", "..", "backend", + "common", "utils", "markdown_utils.py", +) +_md_spec = _ilu.spec_from_file_location("common.utils.markdown_utils", _md_path) +_markdown_utils = _ilu.module_from_spec(_md_spec) +_md_spec.loader.exec_module(_markdown_utils) +sys.modules['common.utils'] = Mock() +sys.modules['common.utils.markdown_utils'] = _markdown_utils + class MockTeamConfiguration: def __init__(self, name="TestTeam", deployment_name="test_deployment"): @@ -991,3 +1004,69 @@ def test_given_new_instance_when_init_then_logger_is_set(self): manager = OrchestrationManager() assert isinstance(manager.logger, logging.Logger) + + +# _normalize_markdown_tables (Bug 47810) +from backend.orchestration.orchestration_manager import ( # noqa: E402 + _normalize_markdown_tables, +) +from common.utils.markdown_utils import ( # noqa: E402 + reflow_collapsed_table_line as _reflow_collapsed_table_line, +) + + +class TestNormalizeMarkdownTables: + """Test markdown table re-flow for collapsed orchestrator output (Bug 47810).""" + + def test_given_collapsed_table_when_normalized_then_rows_split_to_lines(self): + collapsed = ( + "| Risk Type | Description | Rating | " + "|-------|-------|-------| " + "| Delivery | Undefined timeline | Medium | " + "| Financial | Fixed budget | High |" + ) + + result = _normalize_markdown_tables(collapsed) + + lines = [ln for ln in result.split("\n") if ln.strip()] + assert lines == [ + "| Risk Type | Description | Rating |", + "| ------- | ------- | ------- |", + "| Delivery | Undefined timeline | Medium |", + "| Financial | Fixed budget | High |", + ] + + def test_given_collapsed_table_with_prefix_then_prefix_kept_on_own_line(self): + collapsed = ( + "Risk Analysis | A | B | |---|---| | 1 | 2 |" + ) + + result = _normalize_markdown_tables(collapsed) + + # Prefix prose separated from the table by a blank line for GFM. + assert result.startswith("Risk Analysis\n\n| A | B |") + + def test_given_wellformed_table_when_normalized_then_unchanged(self): + good = "| A | B |\n| --- | --- |\n| 1 | 2 |\n| 3 | 4 |" + + assert _normalize_markdown_tables(good) == good + + def test_given_plain_text_when_normalized_then_unchanged(self): + text = "Just some text with a - dash and | a pipe." + + assert _normalize_markdown_tables(text) == text + + def test_given_colon_aligned_delimiter_when_normalized_then_alignment_kept(self): + collapsed = "| A | B | C | |:--|:-:|--:| | 1 | 2 | 3 |" + + result = _normalize_markdown_tables(collapsed) + + assert "| :-- | :-: | --: |" in result + + def test_given_empty_or_none_when_normalized_then_returns_input(self): + assert _normalize_markdown_tables("") == "" + assert _normalize_markdown_tables(None) is None + + def test_given_non_table_pipe_line_when_reflowed_then_returns_none(self): + assert _reflow_collapsed_table_line("a | b | c") is None +