Repository navigation
Query builder: notes follow the user's language - #18
Merged
Merged
Conversation
Reviewer's GuideQuery-builder notes now carry locale keys and interpolation parameters, allowing the shared CLI and GUI rendering paths to display English or Spanish templates while safely falling back to the original English sentence; tests cover translation, integration, and failure handling. Sequence diagram for localized query-builder note renderingsequenceDiagram
participant Composer as QueryComposer
participant Result as QueryResult
participant Renderer as format_result_lines
participant Locale as LocaleTranslator
participant Surface as CLI_or_GUI
Composer->>Result: QueryNote(message, message_key, message_params)
Surface->>Renderer: format_result_lines(result, translate)
Renderer->>Locale: translate(message_key)
alt localized template exists and formats successfully
Locale-->>Renderer: localized template
Renderer-->>Surface: formatted localized note
else key missing or formatting fails
Renderer-->>Surface: note.message
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/auto_apply/src/auto_apply/domain/services/query_composer.py" line_range="337" />
<code_context>
"takes a location.",
+ message_key="builder.note.location_unsupported",
+ surface=spec.display_name,
+ location=request.location,
))
</code_context>
<issue_to_address>
**Location escapes terminal output**
When a scripted CLI invocation supplies an unsupported location containing terminal control sequences, the CLI passes the formatted note to `print`, where terminal control sequences in `request.location` are interpreted, allowing crafted input to forge or hide output and trigger terminal-side actions.
Escape or remove terminal control characters from user-supplied values before printing CLI notes.
Also at `packages/auto_apply/src/auto_apply/domain/models/search_query.py:202`.
</issue_to_address>
### Comment 2
<location path="packages/auto_apply/src/auto_apply/domain/models/search_query.py" line_range="202-203" />
<code_context>
+ template = translate(note.message_key)
+ if template and template != note.message_key:
+ try:
+ return template.format(**dict(note.message_params))
+ except (KeyError, IndexError, ValueError):
+ pass
+ return note.message
</code_context>
<issue_to_address>
**Result rendering fails**
When a locale template uses an attribute-style placeholder whose lookup fails, `template.format(**dict(note.message_params))` raises an uncaught `AttributeError`, so `note_text` cannot return the English fallback and CLI or GUI result rendering fails.
Catch `AttributeError` alongside the existing formatting exceptions so `note_text` returns the English fallback.
</issue_to_address>
### Comment 3
<location path="packages/auto_apply/src/auto_apply/adapters/primary/gui/app.py" line_range="769-770" />
<code_context>
strings = get_strings()
- SearchBuilderWindow(self, builder, translate=lambda key: strings.get(key, key))
+ SearchBuilderWindow(
+ self, builder, translate=lambda key: strings.get(key) or locale_text(key)
+ )
# =====================================================================
</code_context>
<issue_to_address>
**Builder labels and notes diverge**
When the active locale changes while a query-builder window is open, `strings` keeps the window labels in the original locale, while `locale_text` reads the changed global locale for notes, so the window displays labels and notes in different languages.
Make the note translator use the same locale snapshot as the window labels.
Also at `packages/auto_apply/src/auto_apply/adapters/primary/gui/strings.py:30-36`.
</issue_to_address>
### Comment 4
<location path="packages/auto_apply/tests/adapters/test_gui_search_window.py" line_range="180" />
<code_context>
+ labels = {"builder.result.bar": "Texto", "builder.result.link": "Enlace"}
+ try:
+ configure_locale(language="es")
+ lines = gw.result_lines(result, lambda key: labels.get(key) or locale_text(key))
+ finally:
+ configure_locale(language="en") # restore default for other tests
</code_context>
<issue_to_address>
**Broken GUI wiring goes unnoticed**
When the GUI's `_open_search_builder` translator is changed to return untranslated keys for note templates, `test_gui_notes_follow_the_locale_and_never_crash__teeth` passes using the lambda on this line even if the application's GUI translator stops calling `locale_text`; the test calls `gw.result_lines` directly and never exercises `_open_search_builder`, so the real GUI can show English notes.
Exercise `_open_search_builder` and assert that the translator it passes to the window renders a note in Spanish.
</issue_to_address>Sourcery assessment
Approval pending. 4 findings to address first.
Blocking findings: packages/auto_apply/src/auto_apply/domain/services/query_composer.py:337, packages/auto_apply/src/auto_apply/domain/models/search_query.py:203, packages/auto_apply/src/auto_apply/adapters/primary/gui/app.py:770, packages/auto_apply/tests/adapters/test_gui_search_window.py:180
| "takes a location.", | ||
| message_key="builder.note.location_unsupported", | ||
| surface=spec.display_name, | ||
| location=request.location, |
There was a problem hiding this comment.
🟡 Medium · Location escapes terminal output
When a scripted CLI invocation supplies an unsupported location containing terminal control sequences, the CLI passes the formatted note to print, where terminal control sequences in request.location are interpreted, allowing crafted input to forge or hide output and trigger terminal-side actions.
Escape or remove terminal control characters from user-supplied values before printing CLI notes.
Also at packages/auto_apply/src/auto_apply/domain/models/search_query.py:202.
Prompt for AI agents
In `packages/auto_apply/src/auto_apply/domain/services/query_composer.py` at line 337:
**Location escapes terminal output**
When a scripted CLI invocation supplies an unsupported location containing terminal control sequences, the CLI passes the formatted note to `print`, where terminal control sequences in `request.location` are interpreted, allowing crafted input to forge or hide output and trigger terminal-side actions.
Escape or remove terminal control characters from user-supplied values before printing CLI notes.
Also at `packages/auto_apply/src/auto_apply/domain/models/search_query.py:202`.
Comment on lines
+202
to
+203
| return template.format(**dict(note.message_params)) | ||
| except (KeyError, IndexError, ValueError): |
There was a problem hiding this comment.
🟡 Medium · Result rendering fails
When a locale template uses an attribute-style placeholder whose lookup fails, template.format(**dict(note.message_params)) raises an uncaught AttributeError, so note_text cannot return the English fallback and CLI or GUI result rendering fails.
Catch AttributeError alongside the existing formatting exceptions so note_text returns the English fallback.
Prompt for AI agents
In `packages/auto_apply/src/auto_apply/domain/models/search_query.py` at lines 202-203:
**Result rendering fails**
When a locale template uses an attribute-style placeholder whose lookup fails, `template.format(**dict(note.message_params))` raises an uncaught `AttributeError`, so `note_text` cannot return the English fallback and CLI or GUI result rendering fails.
Catch `AttributeError` alongside the existing formatting exceptions so `note_text` returns the English fallback.
Comment on lines
+769
to
+770
| self, builder, translate=lambda key: strings.get(key) or locale_text(key) | ||
| ) |
There was a problem hiding this comment.
🟡 Medium · Builder labels and notes diverge
When the active locale changes while a query-builder window is open, strings keeps the window labels in the original locale, while locale_text reads the changed global locale for notes, so the window displays labels and notes in different languages.
Make the note translator use the same locale snapshot as the window labels.
Also at packages/auto_apply/src/auto_apply/adapters/primary/gui/strings.py:30-36.
Prompt for AI agents
In `packages/auto_apply/src/auto_apply/adapters/primary/gui/app.py` at lines 769-770:
**Builder labels and notes diverge**
When the active locale changes while a query-builder window is open, `strings` keeps the window labels in the original locale, while `locale_text` reads the changed global locale for notes, so the window displays labels and notes in different languages.
Make the note translator use the same locale snapshot as the window labels.
Also at `packages/auto_apply/src/auto_apply/adapters/primary/gui/strings.py:30-36`.| labels = {"builder.result.bar": "Texto", "builder.result.link": "Enlace"} | ||
| try: | ||
| configure_locale(language="es") | ||
| lines = gw.result_lines(result, lambda key: labels.get(key) or locale_text(key)) |
There was a problem hiding this comment.
🟡 Medium · Broken GUI wiring goes unnoticed
When the GUI's _open_search_builder translator is changed to return untranslated keys for note templates, test_gui_notes_follow_the_locale_and_never_crash__teeth passes using the lambda on this line even if the application's GUI translator stops calling locale_text; the test calls gw.result_lines directly and never exercises _open_search_builder, so the real GUI can show English notes.
Exercise _open_search_builder and assert that the translator it passes to the window renders a note in Spanish.
Prompt for AI agents
In `packages/auto_apply/tests/adapters/test_gui_search_window.py` at line 180:
**Broken GUI wiring goes unnoticed**
When the GUI's `_open_search_builder` translator is changed to return untranslated keys for note templates, `test_gui_notes_follow_the_locale_and_never_crash__teeth` passes using the lambda on this line even if the application's GUI translator stops calling `locale_text`; the test calls `gw.result_lines` directly and never exercises `_open_search_builder`, so the real GUI can show English notes.
Exercise `_open_search_builder` and assert that the translator it passes to the window renders a note in Spanish.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Every note the query builder shows — refusals and explanations, on both the CLI and the GUI — now renders in the user's language. Previously every note was an English sentence even when the rest of the screen was Spanish.
Why
AA's rule is that every user-facing string goes through the locale. The composer built its notes as finished English strings, and
format_result_linesprinted them verbatim, so a Spanish user saw Spanish labels next to English notes (measured in the GUI: "Google has no place for a location, so 'Seattle, WA' was left out of the search.").How it was verified
Full suite on Windows:
Summary by Sourcery
Localize query-builder notes across the CLI and GUI while retaining safe English fallbacks.
New Features:
Bug Fixes:
Enhancements:
Tests: