Skip to content

Commit 8407971

Browse files
fix: the RFP team table formatting
2 parents 5219490 + 780049a commit 8407971

6 files changed

Lines changed: 175 additions & 21 deletions

File tree

conftest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,18 @@
1111
agents_path = Path(__file__).parent.parent.parent / "backend" / "v4" / "magentic_agents"
1212
sys.path.insert(0, str(agents_path))
1313

14+
1415
@pytest.fixture
1516
def agent_env_vars():
1617
"""Common environment variables for agent testing."""
1718
return {
1819
"BING_CONNECTION_NAME": "test_bing_connection",
1920
"MCP_SERVER_ENDPOINT": "http://test-mcp-server",
20-
"MCP_SERVER_NAME": "test_mcp_server",
21+
"MCP_SERVER_NAME": "test_mcp_server",
2122
"MCP_SERVER_DESCRIPTION": "Test MCP server",
2223
"TENANT_ID": "test_tenant_id",
2324
"CLIENT_ID": "test_client_id",
2425
"AZURE_OPENAI_ENDPOINT": "https://test.openai.azure.com/",
2526
"AZURE_OPENAI_API_KEY": "test_key",
2627
"AZURE_OPENAI_DEPLOYMENT_NAME": "test_deployment"
27-
}
28+
}

src/backend/api/router.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1466,4 +1466,4 @@ async def get_generated_image(blob_name: str):
14661466
return Response(content=data, media_type="image/png")
14671467
except Exception as exc:
14681468
logging.error(f"Error retrieving image '{blob_name}': {exc}")
1469-
raise HTTPException(status_code=404, detail="Image not found")
1469+
raise HTTPException(status_code=404, detail="Image not found")

src/backend/callbacks/response_handlers.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
AgentToolCall, AgentToolMessage,
1414
WebsocketMessageType)
1515
from orchestration.connection_config import connection_config
16+
from common.utils.markdown_utils import normalize_markdown_tables
1617

1718
logger = logging.getLogger(__name__)
1819

@@ -113,6 +114,9 @@ def agent_response_callback(
113114

114115
text = clean_citations(text or "")
115116

117+
# Repair collapsed markdown tables before rendering (Bug 47810).
118+
text = normalize_markdown_tables(text)
119+
116120
if not user_id:
117121
logger.debug("No user_id provided; skipping websocket send for final message.")
118122
return
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
"""Shared GFM table repair utilities (Bug 47810)."""
2+
3+
import re
4+
from typing import List, Optional
5+
6+
# Matches a GFM delimiter cell: "---", ":--", "--:", ":-:".
7+
_TABLE_DELIM_CELL_RE = re.compile(r"^\s*:?-+:?\s*$")
8+
9+
10+
def reflow_collapsed_table_line(line: str) -> Optional[List[str]]:
11+
"""Rebuild a GFM table flattened onto one line; return None if not collapsed."""
12+
if line.count("|") < 4 or "-" not in line:
13+
return None
14+
15+
first = line.index("|")
16+
prefix = line[:first].rstrip()
17+
raw = line[first:].rstrip()
18+
19+
tokens = raw.split("|")
20+
if tokens and tokens[0].strip() == "":
21+
tokens = tokens[1:]
22+
if tokens and tokens[-1].strip() == "":
23+
tokens = tokens[:-1]
24+
if not tokens:
25+
return None
26+
27+
# Whitespace-only tokens mark the boundary between flattened rows.
28+
rows: List[List[str]] = []
29+
current: List[str] = []
30+
for tok in tokens:
31+
if tok.strip() == "":
32+
if current:
33+
rows.append(current)
34+
current = []
35+
else:
36+
current.append(tok)
37+
if current:
38+
rows.append(current)
39+
40+
# Require a header plus a delimiter row; leaves well-formed tables untouched.
41+
if len(rows) < 2 or not all(_TABLE_DELIM_CELL_RE.match(c) for c in rows[1]):
42+
return None
43+
44+
n = len(rows[1])
45+
if n == 0 or len(rows[0]) != n:
46+
return None
47+
48+
rendered = ["| " + " | ".join(cell.strip() for cell in row) + " |" for row in rows]
49+
50+
result: List[str] = []
51+
if prefix:
52+
result.append(prefix)
53+
result.append("") # Blank line so GFM starts a fresh table block.
54+
result.extend(rendered)
55+
return result
56+
57+
58+
def normalize_markdown_tables(text: str) -> str:
59+
"""Repair collapsed GFM tables; non-table text is returned unchanged."""
60+
if not text or "|" not in text or "-" not in text:
61+
return text
62+
63+
out: List[str] = []
64+
for line in text.split("\n"):
65+
reflowed = reflow_collapsed_table_line(line)
66+
if reflowed is None:
67+
out.append(line)
68+
else:
69+
out.extend(reflowed)
70+
return "\n".join(out)

src/backend/orchestration/orchestration_manager.py

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
from common.config.app_config import config
2222
from common.database.database_base import DatabaseBase
2323
from common.models.messages import TeamConfiguration
24+
from common.utils.markdown_utils import \
25+
normalize_markdown_tables as _normalize_markdown_tables
2426
from models.messages import AgentMessageStreaming, WebsocketMessageType
2527
from orchestration.connection_config import (connection_config,
2628
orchestration_config)
@@ -36,18 +38,18 @@
3638
apply_tool_history_leak_patch()
3739

3840
_BARE_IMAGE_URL_RE = re.compile(
39-
r"(?<![\(\]])"
40-
r"(?<!\]\()"
41-
r"("
42-
# Absolute image URL (any host, or a backend /api/v4/images path)
43-
r"https?://[^\s)]+?(?:/api/v4/images/[^\s)]+?|[^\s)]+?\.(?:png|jpe?g|gif|webp))"
44-
# Bare relative backend image path (emitted by the MCP/backend image tools).
45-
# The (?<![^\s]) guard requires the path to start at whitespace/string-start so
46-
# it never matches the same substring inside an absolute URL.
47-
r"|(?<![^\s])/api/v4/images/[^\s)]+?\.(?:png|jpe?g|gif|webp)"
48-
r")"
49-
r"(?=[\s)\]]|$)",
50-
re.IGNORECASE,
41+
r"(?<![\(\]])"
42+
r"(?<!\]\()"
43+
r"("
44+
# Absolute image URL (any host, or a backend /api/v4/images path)
45+
r"https?://[^\s)]+?(?:/api/v4/images/[^\s)]+?|[^\s)]+?\.(?:png|jpe?g|gif|webp))"
46+
# Bare relative backend image path (emitted by the MCP/backend image tools).
47+
# The (?<![^\s]) guard requires the path to start at whitespace/string-start so
48+
# it never matches the same substring inside an absolute URL.
49+
r"|(?<![^\s])/api/v4/images/[^\s)]+?\.(?:png|jpe?g|gif|webp)"
50+
r")"
51+
r"(?=[\s)\]]|$)",
52+
re.IGNORECASE,
5153
)
5254

5355

@@ -231,25 +233,20 @@ async def get_current_or_new_orchestration(
231233
current is not None and current_team_id != team_config.team_id
232234
)
233235

234-
235236
cls.logger.info(
236237
"get_current_or_new_orchestration: user='%s' selected_team='%s' "
237238
"cached_team='%s' team_switched=%s team_changed=%s current_is_none=%s",
238239
user_id, team_config.team_id, current_team_id,
239240
team_switched, team_changed, current is None,
240241
)
241242

242-
243243
# Full rebuild: no workflow exists, team explicitly switched, or the
244244
# cached workflow belongs to a different team than the selected one.
245245
needs_full_rebuild = current is None or team_switched or team_changed
246246

247-
248247
# Lightweight reset: workflow finished but agents are still valid for the
249248
# same team (a team change always routes to full rebuild above so we
250249
# never reuse the previous team's agents here).
251-
252-
253250
needs_workflow_reset = not needs_full_rebuild and workflow_terminated
254251

255252
if needs_full_rebuild:
@@ -455,6 +452,9 @@ async def run_orchestration(self, user_id: str, input_task) -> None:
455452
# accumulated orchestrator streaming chunks.
456453
final_text = final_output_ref[0] or "".join(orchestrator_chunks)
457454

455+
# Repair collapsed markdown tables before rendering (Bug 47810).
456+
final_text = _normalize_markdown_tables(final_text)
457+
458458
final_text = _embed_bare_image_urls(final_text)
459459

460460
# Issue 1 diagnostic: confirm the final answer carries a renderable image
@@ -865,4 +865,4 @@ async def _process_event_stream(
865865
if tool_approvals:
866866
result["tool_approvals"] = tool_approvals
867867
return result
868-
return None
868+
return None

src/tests/backend/orchestration/test_orchestration_manager.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,19 @@ def _make_workflow_mock(run_return=None, executors=None):
177177
sys.modules['common.config.app_config'] = Mock(config=mock_config)
178178
sys.modules['common.models'] = Mock()
179179

180+
# Register the real markdown_utils so the orchestrator uses genuine table logic, not a Mock (Bug 47810).
181+
import importlib.util as _ilu # noqa: E402
182+
183+
_md_path = os.path.join(
184+
os.path.dirname(__file__), "..", "..", "..", "backend",
185+
"common", "utils", "markdown_utils.py",
186+
)
187+
_md_spec = _ilu.spec_from_file_location("common.utils.markdown_utils", _md_path)
188+
_markdown_utils = _ilu.module_from_spec(_md_spec)
189+
_md_spec.loader.exec_module(_markdown_utils)
190+
sys.modules['common.utils'] = Mock()
191+
sys.modules['common.utils.markdown_utils'] = _markdown_utils
192+
180193

181194
class MockTeamConfiguration:
182195
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):
9911004
manager = OrchestrationManager()
9921005

9931006
assert isinstance(manager.logger, logging.Logger)
1007+
1008+
1009+
# _normalize_markdown_tables (Bug 47810)
1010+
from backend.orchestration.orchestration_manager import ( # noqa: E402
1011+
_normalize_markdown_tables,
1012+
)
1013+
from common.utils.markdown_utils import ( # noqa: E402
1014+
reflow_collapsed_table_line as _reflow_collapsed_table_line,
1015+
)
1016+
1017+
1018+
class TestNormalizeMarkdownTables:
1019+
"""Test markdown table re-flow for collapsed orchestrator output (Bug 47810)."""
1020+
1021+
def test_given_collapsed_table_when_normalized_then_rows_split_to_lines(self):
1022+
collapsed = (
1023+
"| Risk Type | Description | Rating | "
1024+
"|-------|-------|-------| "
1025+
"| Delivery | Undefined timeline | Medium | "
1026+
"| Financial | Fixed budget | High |"
1027+
)
1028+
1029+
result = _normalize_markdown_tables(collapsed)
1030+
1031+
lines = [ln for ln in result.split("\n") if ln.strip()]
1032+
assert lines == [
1033+
"| Risk Type | Description | Rating |",
1034+
"| ------- | ------- | ------- |",
1035+
"| Delivery | Undefined timeline | Medium |",
1036+
"| Financial | Fixed budget | High |",
1037+
]
1038+
1039+
def test_given_collapsed_table_with_prefix_then_prefix_kept_on_own_line(self):
1040+
collapsed = (
1041+
"Risk Analysis | A | B | |---|---| | 1 | 2 |"
1042+
)
1043+
1044+
result = _normalize_markdown_tables(collapsed)
1045+
1046+
# Prefix prose separated from the table by a blank line for GFM.
1047+
assert result.startswith("Risk Analysis\n\n| A | B |")
1048+
1049+
def test_given_wellformed_table_when_normalized_then_unchanged(self):
1050+
good = "| A | B |\n| --- | --- |\n| 1 | 2 |\n| 3 | 4 |"
1051+
1052+
assert _normalize_markdown_tables(good) == good
1053+
1054+
def test_given_plain_text_when_normalized_then_unchanged(self):
1055+
text = "Just some text with a - dash and | a pipe."
1056+
1057+
assert _normalize_markdown_tables(text) == text
1058+
1059+
def test_given_colon_aligned_delimiter_when_normalized_then_alignment_kept(self):
1060+
collapsed = "| A | B | C | |:--|:-:|--:| | 1 | 2 | 3 |"
1061+
1062+
result = _normalize_markdown_tables(collapsed)
1063+
1064+
assert "| :-- | :-: | --: |" in result
1065+
1066+
def test_given_empty_or_none_when_normalized_then_returns_input(self):
1067+
assert _normalize_markdown_tables("") == ""
1068+
assert _normalize_markdown_tables(None) is None
1069+
1070+
def test_given_non_table_pipe_line_when_reflowed_then_returns_none(self):
1071+
assert _reflow_collapsed_table_line("a | b | c") is None
1072+

0 commit comments

Comments
 (0)