Skip to content

Remove rephrase API endpoints - #189

Merged
jmsevin merged 3 commits into
mainfrom
remove-obsolete-endpoints
Sep 7, 2026
Merged

jmsevin merged 3 commits into
mainfrom
remove-obsolete-endpoints

Conversation

@jmsevin

@jmsevin jmsevin commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

This pull request removes all support for the chat rephrasing feature from the codebase. This includes deleting the related REST API endpoints, service methods, prompt templates, and associated tests. The main goal is to simplify the code by removing unused or deprecated functionality.

The most important changes are:

API Endpoint Removal

  • Removed the /api/v1/qna/chat/rephrase and /api/v1/qna/chat/rephrase_stream REST API endpoints and their documentation from src/app/api/api_v1/endpoints/chat.py and README.md. [1] [2]

Service Layer Cleanup

  • Deleted the rephrase_message method from the AbstractChat service class in src/app/shared/infra/abst_chat.py.
  • Removed the REPHRASE prompt template from src/app/services/prompts.py.

Model and Type Cleanup

  • Removed the PROMPTS literal type, which included "REPHRASE", from src/app/models/chat.py.

Test Suite Cleanup

  • Deleted all tests related to the chat rephrase feature from src/app/tests/api/api_v1/test_chat.py and src/app/tests/services/test_abst_chat.py. [1] [2]

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The removal of rephrase_message also dropped the @traceable decorator from chat_message, reducing production observability and likely unintentionally changing tracing behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This pull request removes the deprecated chat “rephrase” feature from the API layer, service layer, prompt templates, models, documentation, and related tests to simplify the codebase.

Changes:

  • Removed /api/v1/qna/chat/rephrase and /api/v1/qna/chat/rephrase_stream endpoints and README documentation.
  • Deleted AbstractChat.rephrase_message and the REPHRASE prompt template; removed the PROMPTS literal type that included "REPHRASE".
  • Removed tests covering the rephrase endpoints/service behavior.
File summaries
File Description
src/app/tests/services/test_abst_chat.py Removes unit tests for the deleted rephrase_message behavior.
src/app/tests/api/api_v1/test_chat.py Removes API tests for the deleted rephrase endpoint.
src/app/shared/infra/abst_chat.py Removes rephrase_message implementation from the chat service layer.
src/app/services/prompts.py Removes the REPHRASE prompt template.
src/app/models/chat.py Removes PROMPTS literal type that referenced "REPHRASE".
src/app/api/api_v1/endpoints/chat.py Removes rephrase API routes and related typing import.
README.md Removes documentation entries for the rephrase endpoints.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/app/shared/infra/abst_chat.py
@jmsevin
jmsevin merged commit 480b0c9 into main Sep 7, 2026
3 checks passed
@jmsevin
jmsevin deleted the remove-obsolete-endpoints branch September 7, 2026 13:23
noor-lpi pushed a commit that referenced this pull request Sep 10, 2026
abst_chat.py still had literal unresolved merge-conflict markers
around the langsmith/langgraph import block from the rebase onto
main — both imports were actually needed (traceable from main's newer
tracing instrumentation, REMOVE_ALL_MESSAGES for the persisted
tool-clearing middleware), so this just keeps both lines and drops
the markers.

test_abst_chat.py still imported ReformulatedQueryResponse, which
main's "Remove rephrase API endpoints" (#189) deleted along with the
rephrase/reformulate tests that used it — the tests were already gone
from this branch, just the now-dead import survived. Removed it.

Both were breaking the module import outright (SyntaxError / ImportError).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sandragjacinto added a commit that referenced this pull request Sep 10, 2026
* feat(chat): iteration 1 — rewrite prompts and fix language detection

Rewrites all prompt templates to be cleaner and more instruction-precise
(AGENT_SYSTEM_PROMPT, SYSTEM_PROMPT, SOURCED_ANSWER, REPHRASE,
GENERATE_NEW_QUESTIONS, reformulate/standalone prompts).

Fixes get_new_questions() to actually use the detected language when
formatting the GENERATE_NEW_QUESTIONS template (was previously ignored),
and corrects history slicing from broken [::-2][:2] to [-2:].

Fixes reformulate_user_query() to call the LLM via run_llm_with_json_parsing
instead of returning a hardcoded stub.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(chat): iteration 2 — citation grounding, language fix, multi-tool-call fix

AGENT_SYSTEM_PROMPT: anti-sycophancy, stronger length limit (1-2 sentences
for openers), one tool call per response with comprehensive query, explicit
prohibition on invented facts/examples, character-for-character URL copy
instruction.

Language detection (get_new_questions): when history exists, detect language
from the last 4 history messages instead of the current short query (more
reliable for langdetect). Accept optional ui_language (lang) param for
the empty-chat case where there is no query to detect from. Thread lang
through Context → ContextOut → endpoint → service.

Agent sources (agent_response): collect artifacts from ALL ToolMessages
instead of only the last one, so the right panel shows the complete set
of retrieved docs when the agent makes multiple tool calls.

Agent iteration cap (agent_message): add recursion_limit=5 to RunnableConfig
to prevent runaway multi-call loops; complements the prompt instruction.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: apply black formatting across codebase

Removes blank lines after function/class definitions in several files,
applied automatically by black during lint run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Revert "fix(chat): enforce one-shot tool call and preserve docs panel on follow-up"

This reverts commit 7bdcfc7.

* Reapply "fix(chat): enforce one-shot tool call and preserve docs panel on follow-up"

This reverts commit c8a562c.

* fix(chat): reorient follow-up questions to WeLearn's pedagogical use case

Both the agent's own end-of-turn question and the separate
/reformulate/questions suggestions were framed generically around
"continuing to learn about SDGs" rather than WeLearn's actual
purpose: helping professors and course designers move from
understanding sustainability topics to applying them in their own
teaching.

* fix(chat): auto-link citations the model forgets to format

Citations occasionally rendered as plain "[Doc N]" text instead of
clickable links when the model didn't follow the formatting
instruction. Adds a regex-based safety net (linkify_missing_citations
in helpers.py) that wraps any bare marker with the correct <a> tag
using the URL already available from the retrieved docs, applied to
both the streaming and non-streaming agent endpoints.

* fix(tutor): avoid same-quote f-string nesting for Python <3.12 compatibility

Nested f-strings reusing the outer quote character only became legal
in Python 3.12. Extracting the joined string into a variable first
keeps this working on earlier versions.

* fix(chat): remove ambiguity from agent system prompt, strengthen citation rules

Team review flagged several ambiguous references in AGENT_SYSTEM_PROMPT
("in this same conversation turn", "(see above)", "the retrieval tool")
and asked for harder "never" language plus repeated/reinforced citation
rules. Also adds a new section so requests for a specific deliverable
(e.g. a detailed learning activity) don't fall back to overly long,
externally-linked output.

* fix(agents): correct formatting of disciplinary skills sentences in generate method

* commented lines 29 to 39 and line 44 to ignore unit tests failing

* fix(chat): keep only latest tool results, curb citation drift, fight context rot

Only the most recent get_resources_about_sustainability call's documents stay
in the checkpointed conversation history; older ones are permanently cleared
(not just hidden per-call) before summarization runs, so summarization no
longer has to compress bulk that's already gone. Lowered the summarization
trigger since the model was starting to ignore its own system prompt in long
threads, and added a middleware that re-states a condensed version of the
full prompt on every turn to keep those instructions salient. Also allows
citing the same retrieved docs across turns when the topic hasn't shifted,
and adds a vouvoiement rule for French replies.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(chat): tighten citation rules and switch to Markdown link format

External sources were sometimes named (title/journal) without being
retrieved, since the prompt explicitly allowed unlinked plain-text
mentions of non-retrieved sources — now forbidden outright. Combined
citation markers like "[Docs 3 et 5]" are unlinkable (no single URL to
resolve to), so the prompt now requires one document per marker, and
the model is told to verify a claim is actually in the document it
cites.

Also switch citation format from HTML anchors to Markdown links
([Doc N](URL)), matching the rest of a Markdown reply and reducing
formatting mistakes. linkify_missing_citations follows suit, but note
it only patches the non-streaming endpoint and the saved chat record —
it never reaches the live streamed view, so the prompt rules are the
only real fix for what streams to the user.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(chat): use double-bracket citation markers so brackets survive Markdown rendering

A plain Markdown link [Doc N](URL) renders with its brackets and URL
consumed as syntax, showing only the bare text "Doc N" — this is
correct Markdown behavior (per CommonMark), not the model dropping the
citation. Since the brackets need to stay visible, wrap the whole
marker in an extra pair: [[Doc N]](URL). The outer brackets are the
link syntax (invisible once rendered), the inner "[Doc N]" is the
literal label that survives rendering.

Updated AGENT_SYSTEM_PROMPT and AGENT_REMINDER_PROMPT with the
double-bracket format and a concrete wrong/correct example, and
linkify_missing_citations's fallback wrapping to match, without
re-wrapping a marker that's already correctly double-bracketed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: correct gitignore entries for DS_Store

* uncommented lines 29-39 and 44 for tests to run

* fix: clean up rebase leftovers (stray conflict markers, dead import)

abst_chat.py still had literal unresolved merge-conflict markers
around the langsmith/langgraph import block from the rebase onto
main — both imports were actually needed (traceable from main's newer
tracing instrumentation, REMOVE_ALL_MESSAGES for the persisted
tool-clearing middleware), so this just keeps both lines and drops
the markers.

test_abst_chat.py still imported ReformulatedQueryResponse, which
main's "Remove rephrase API endpoints" (#189) deleted along with the
rephrase/reformulate tests that used it — the tests were already gone
from this branch, just the now-dead import survived. Removed it.

Both were breaking the module import outright (SyntaxError / ImportError).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* style: apply black formatting to abst_chat.py and test_abst_chat.py

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix test mock

* review prompt for formality

---------

Co-authored-by: Noor A <Noor@MacBook-Pro-de-CRI.local>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Théo <theo.nardin@cri-paris.org>
Co-authored-by: Sandra Guerreiro <sandragjacinto@gmail.com>
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.

3 participants