Skip to content

Get rid of Emojis #193 - #204

Open
jmacd867 wants to merge 2 commits into
mainfrom
RemoveEmojis
Open

jmacd867 wants to merge 2 commits into
mainfrom
RemoveEmojis

Conversation

@jmacd867

Copy link
Copy Markdown
Collaborator

Removed emojis that caused issues in #193

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

inference/README.md still documents the removed Unicode encoding issue and workaround.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Low severity

Open (1)
What changed in this PR

This PR removes Unicode status emojis from relay output and inference-script logs to avoid Windows encoding failures when output is redirected or piped.

Changes:

  • Replaces emoji markers with ASCII tags in Rust relay messages.
  • Replaces emoji markers in inference logging and output.
  • Updates the relay output example in the root README.
File Description
relay/​src/​view.rs Uses an ASCII warning marker in the UI.
relay/​src/​state.rs Uses ASCII success/error markers for listener status.
README.md Updates the relay output example.
inference/​train_model.py Uses ASCII markers for training and save messages.
inference/​test_model.py Uses ASCII markers throughout testing output.
inference/​label_generator.py Uses ASCII markers and removes arrow-comment characters.

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

Comment thread inference/test_model.py

self.model = joblib.load(model_path)
logger.info(f"✓ Model loaded successfully: {type(self.model).__name__}")
logger.info(f"[OK] Model loaded successfully: {type(self.model).__name__}")
@Phlabry

Phlabry commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

LGTM overall. No non-ASCII left on the branch, README samples match the new relay output, CI green.

One thing before I approve:

  • The second commit drops two troubleshooting bullets from inference/README.md, not one. Removing the UnicodeEncodeError one is right; the "ValueError: Missing required columns" one below it is unrelated and still reachable (prepare_data.py:90, train_model.py:73). Can you put that back?

Nits, ignore if you want:

  • Prettier-on-save reformatted the table and list spacing, which buries the actual change in noise. Worth turning off format-on-save for markdown so the diff stays limited to what you meant to change.
  • The "--project inference" continuation line lost its indent.

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.

inference scripts crash on Windows when output is redirected or piped

3 participants