Skip to content

docs+test: gateway quickstart section + regression tests for issue #3#7

Open
dengluozhang-png wants to merge 1 commit into
lawcontinue:mainfrom
dengluozhang-png:docs/gateway-section
Open

docs+test: gateway quickstart section + regression tests for issue #3#7
dengluozhang-png wants to merge 1 commit into
lawcontinue:mainfrom
dengluozhang-png:docs/gateway-section

Conversation

@dengluozhang-png

Copy link
Copy Markdown

Summary

Follow-up on issue #3 (closed in commit 9e7878c without regression tests) and
adds user-facing gateway quickstart documentation that was missing from the
README.

This is a small, low-risk PR: 2 files changed, 21 new + 4 modified tests all
green, no source code changes outside tests/.

Changes

README.md — expand "Start the API server" section

The existing one-line snippet (python -m opensymphony.gateway.http) left
users without:

  1. How to customize host/port
  2. How to use uvicorn directly (production-style)
  3. What endpoints exist (the gateway has 30+ handlers; README mentioned none)
  4. Which env vars configure which LLM providers

This PR adds three subsections under Quick Start:

  • Custom host/port via GATEWAY_HOST / GATEWAY_PORT env vars
  • uvicorn opensymphony.gateway.http:create_app --factory --host ... --port ...
  • Gateway Endpoints table (8 representative routes)
  • Provider configuration block (MIMO/DEEPSEEK/MOONSHOT/ZHIPU/MINIMAX/OPENAI + optional *_MODEL override)

No source-code change required; all env vars are already wired in
opensymphony/llm/router.py:create_router_from_env() — this just documents
them.

tests/test_tool_use.py — 4 regression tests for issue #3 fix

Issue #3 (_parse_tool_call silently drops tool calls with invalid JSON
escapes) was fixed in commit 9e7878c with an inline regex fallback inside
agent.py. The fix shipped without tests. This PR closes that gap.

def test_parse_tool_call_windows_path(self):
    """Regression: raw Windows path with single backslashes
    (e.g. \\Users\\foo) should be repaired, not silently dropped."""

def test_parse_tool_call_preserves_valid_escapes(self):
    """Regression: valid JSON escapes (\\n, \\t, \\\\) must round-trip
    through the repair step unchanged."""

def test_parse_tool_call_unicode_escape_preserved(self):
    """Regression: \\uXXXX escapes are valid and must not be double-escaped
    by the repair regex."""

def test_parse_tool_call_rejects_total_garbage(self):
    """Regression: after repair, truly invalid JSON still returns None
    (no crash, no silent success)."""

Each test exercises the exact failure mode described in the original issue
(Windows paths in this case; regex strings \d / \w are also covered by
preserves_valid_escapes).

Test results

tests/test_tool_use.py ...................................   [100%]
============================= 21 passed in 0.67s ==============================

No regressions introduced. The new tests pass on commit 9e7878c without any
source changes — i.e. they confirm the existing fix works.

Why not push a PR with more changes

We also have local additions that are not upstream-suitable:

  • SearchCorpusTool (and its TOOL_SCHEMAS entry + register_all() call) —
    personal tooling, may not fit upstream's roadmap.
  • _swap_mimo_for_minimax personal-deployment fallback in
    opensymphony/llm/router.py — a patch for one specific user's Mimo →
    MiniMax migration, not a generalizable change.

Those will stay as local patches. This PR is the minimum useful contribution:
better README + missing tests for the upstream fix.

Related



- README: expand 'Start the API server' with custom host/port,
  uvicorn example, endpoint table, provider env vars
- tests/test_tool_use.py: add 4 regression tests for _parse_tool_call
  invalid-escape fallback:
    * test_parse_tool_call_windows_path
    * test_parse_tool_call_preserves_valid_escapes
    * test_parse_tool_call_unicode_escape_preserved
    * test_parse_tool_call_rejects_total_garbage
  Closes follow-up on issue lawcontinue#3 (which was fixed in 9e7878c without tests).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant