Skip to content

Commit 406d589

Browse files
torrid-fishclaude
andcommitted
fix(accent): address Copilot review feedback (refs #52)
Tighten the no-behavior-change refactor based on Copilot's review on PR #52. Active findings: - pipeline.py: rename unused `ojad_surface` to `_ojad_surface` so F841 catches it if Ruff's unused-binding rule ever lands; the OJAD echo string isn't consumed here. - furigana.py: wrap `response.json()` in try/except. The docstring promised malformed payloads would surface via the FuriganaResponse envelope, but an invalid Content-Type / non-JSON body would have raised through. Catch ValueError and return a 500 envelope. - ojad.py: switch `raise e` -> bare `raise` and `logger.error(f"...")` -> `logger.exception(...)` to preserve the original traceback. - models.py: "describe" -> "describes" (x3 occurrences); "givent" -> "given". Low-confidence findings also addressed: - align.py module docstring used to claim `punctuation_marks`, `skip_marks`, and `clean_query` are consumed by alignment. They aren't — they're carried over from the pre-refactor module for the downstream PR #47 to use. Reword to reflect that. - clean_query docstring overclaimed punctuation stripping; it only filters ASCII letters. Reword + rename the local comprehension var from `chr` to `char` to stop shadowing the builtin. Verified: uv run ruff check api/accent/ main.py # all passed uv run ruff format --check api/accent/ main.py # 8/8 formatted uv run mypy api/accent/ main.py # 8 files, no issues POST /api/MarkAccent/ + /MarkFurigana/ routes still register Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e625782 commit 406d589

5 files changed

Lines changed: 27 additions & 15 deletions

File tree

api/accent/align.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77
token's reading); kana / kanji tokens use literal length + content
88
matching under kata2hira folding.
99
10-
The constants `punctuation_marks`, `skip_marks`, and `numeric_pattern`
11-
plus the `clean_query` / `is_kana_or_kanji` helpers live here because
12-
they're consumed by alignment.
10+
Alignment itself uses `numeric_pattern` and `is_kana_or_kanji`. The
11+
adjacent `punctuation_marks`, `skip_marks`, and `clean_query` are
12+
carried over from the pre-refactor module as part of the accent
13+
domain vocabulary and are kept here for downstream PRs (see #47).
1314
"""
1415

1516
from __future__ import annotations
@@ -105,10 +106,13 @@
105106

106107

107108
def clean_query(query: str) -> str:
108-
"""For OJAD, the query text should without punctuations and alphabets
109-
for better result.
109+
"""Strip ASCII letters from `query`.
110+
111+
OJAD's CRF parser gives better results when Latin alphabet runs are
112+
removed before submission. Punctuation is intentionally left in
113+
place — OJAD relies on it for phrase boundaries.
110114
"""
111-
return "".join(chr for chr in query if chr not in skip_marks)
115+
return "".join(char for char in query if char not in skip_marks)
112116

113117

114118
def is_kana_or_kanji(char: Any) -> bool:

api/accent/furigana.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,15 @@ async def fetch_furigana(text: str, client: httpx.AsyncClient) -> FuriganaRespon
6262
),
6363
)
6464

65-
result = response.json()
65+
try:
66+
result = response.json()
67+
except ValueError as e:
68+
return FuriganaResponse(
69+
status=500,
70+
result=None,
71+
error=ErrorInfo(code=500, message=f"Yahoo API returned invalid JSON: {e}"),
72+
)
73+
6674
if "result" not in result or "word" not in result["result"]:
6775
return FuriganaResponse(
6876
status=500,

api/accent/models.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class ErrorInfo(BaseModel):
2626

2727
code: int = Field(description="The error code that follows JSON-RPC 2.0")
2828
message: str = Field(
29-
description="The error message that describe the details of an error"
29+
description="The error message that describes the details of an error"
3030
)
3131

3232

@@ -75,7 +75,7 @@ class WordAccentResult(BaseModel):
7575

7676
furigana: str = Field(description="Furigana of given kana and kanji")
7777
surface: str = Field(description="The (partial of) original query text")
78-
accent: list[AccentInfo] = Field(description="The accent of givent word")
78+
accent: list[AccentInfo] = Field(description="The accent of given word")
7979
subword: list[WordResult] = Field(
8080
default_factory=list,
8181
description="A list contains more details when a word contains "
@@ -94,7 +94,7 @@ class FuriganaResponse(BaseModel):
9494
)
9595
error: ErrorInfo | None = Field(
9696
default=None,
97-
description="An object that describe the details of an error when occur",
97+
description="An object that describes the details of an error when one occurs",
9898
)
9999

100100

@@ -109,5 +109,5 @@ class AccentResponse(BaseModel):
109109
)
110110
error: ErrorInfo | None = Field(
111111
default=None,
112-
description="An object that describe the details of an error when occur",
112+
description="An object that describes the details of an error when one occurs",
113113
)

api/accent/ojad.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ async def get_ojad_result(
5050
response = await client.post(OJAD_URL, data=data)
5151
response.raise_for_status()
5252
logger.debug(f"[OJAD] Status Code: {response.status_code}")
53-
except Exception as e:
54-
logger.error(f"[OJAD] Request Failed: {e}")
55-
raise e
53+
except Exception:
54+
logger.exception("[OJAD] Request Failed")
55+
raise
5656

5757
website = response.text
5858

api/accent/pipeline.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ async def process_accent_chunk(text: str, client: httpx.AsyncClient) -> AccentRe
4242
furigana_results = furigana_response.result
4343
logger.debug(f"Yahoo Results Count: {len(furigana_results)}")
4444

45-
ojad_surface, ojad_results = await get_ojad_result(query_text, client)
45+
_ojad_surface, ojad_results = await get_ojad_result(query_text, client)
4646

4747
final_results = await align_accent(furigana_results, ojad_results)
4848

0 commit comments

Comments
 (0)