fix(legacy_aliases): tolerate unquoted keys + skip comments in alias-table parser

[sanil-23]

Summary

Fix the core::legacy_aliases::tests::frontend_legacy_aliases_match_server_alias_table test, which #2853 broke by adding health_snapshot: CORE_RPC_METHODS.healthSnapshot, to LEGACY_METHOD_ALIASES.

Why the test was broken

Prettier strips quotes from object keys that are valid JS identifiers, so 'health_snapshot': round-trips to health_snapshot:. The Rust test parser's parse_frontend_legacy_aliases then called quoted_value() on the bare identifier and panicked:

expected quoted value in `health_snapshot`

My first attempt was the obvious one — quote the key in rpcMethods.ts. CI caught the next layer: Prettier's format:check rejects the redundant quotes. So the fix has to be in the parser, not the source file.

What this PR changes

src/core/legacy_aliases.rs — parse_frontend_legacy_aliases only:

1. Skip //-comment lines when compacting the LEGACY_METHOD_ALIASES body. Without this the first quoted key picks up the inline comment header (regression in the prior attempt; caught by full test run).
2. Accept both quoted and bare-identifier keys. 'foo': continues to work; bare foo: (valid JS identifier) is now accepted too. Detection is by leading ' or ".

Impact

This unblocks every PR rebasing onto current main. Confirmed downstream effects:

• feat(intelligence): add architecture diagram viewer #2687 (architecture diagram viewer): 2 fails (test / Rust Core Tests + Quality + Rust Core Coverage (cargo-llvm-cov)) — both this root cause.
• feat(tool_policy): diagnostics RPC and Developer Options panel #2715 (tool_policy diagnostics): same 2 fails after rebase.

Test plan

• All 20 core::legacy_aliases::tests::* pass locally (20 passed; 0 failed).
• app/src/services/rpcMethods.ts left untouched — no Prettier regression.
• CI green on this PR before merge.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Improved robustness of legacy-alias parsing tests: now tolerate comment lines and accept legacy keys written as bare identifiers or quoted strings, with added coverage to assert these behaviors.

[coderabbitai]

Warning

Review limit reached

@sanil-23, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 42 minutes and 39 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5acc8424-6abc-46da-94ae-38a4e4a92233

📥 Commits

Reviewing files that changed from the base of the PR and between 857c389 and 2238e71.

📒 Files selected for processing (1)

• src/core/legacy_aliases.rs

📝 Walkthrough

Walkthrough

The Rust test helper in src/core/legacy_aliases.rs now skips // comment lines when compacting the frontend LEGACY_METHOD_ALIASES body and accepts legacy keys as either quoted strings or bare JavaScript identifiers, normalizing both before resolving aliases. A unit test covers these behaviors.

Changes

Legacy Alias Parser Robustness

Layer / File(s)	Summary
Comment filtering and bare identifier key handling src/core/legacy_aliases.rs	Compacting the LEGACY_METHOD_ALIASES object now filters // comment lines as well as blank lines. The alias-entry parser accepts quoted string keys ('foo':) and bare JS identifier keys (foo:), converting bare identifiers to string keys before resolving mapped targets. A unit test verifies both behaviors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• Missing legacy alias for health_snapshot RPC method #2852: Changes touch the same src/core/legacy_aliases.rs parsing behavior and address comment/bare-identifier handling discussed in that issue.

Possibly related PRs

• tinyhumansai/openhuman#1705: Related edits to src/core/legacy_aliases.rs around alias parsing/normalization.
• tinyhumansai/openhuman#2853: Addresses bare identifier legacy alias usage validated by the improved parser.

Suggested reviewers

• graycyrus

Poem

🐰 I nibbled lines of code so neat,

Skipping comments, keys now meet,
Quoted or bare, they all align,
Tests hop through and say "it's fine!" 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: fixing the legacy_aliases parser to tolerate unquoted keys and skip comments, which directly addresses the core issue described in the PR objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/legacy_aliases.rs`:
- Around line 238-245: Add a unit test to cover bare identifier keys for the new
parsing logic: create a test (e.g.,
parse_frontend_legacy_aliases_accepts_bare_identifier_keys) that builds a source
string containing CORE_RPC_METHODS and LEGACY_METHOD_ALIASES with both a bare
key (health_snapshot: CORE_RPC_METHODS.healthSnapshot) and a quoted key, call
parse_core_rpc_methods and parse_frontend_legacy_aliases, and assert that
aliases.get("health_snapshot") maps to "openhuman.health_snapshot" and the
quoted key still resolves; this ensures the legacy_trimmed/quoted_value branch
handling of bare identifiers is exercised alongside the existing
parse_frontend_legacy_aliases_resolves_core_method_refs_and_literals test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d643bae3-b633-47b8-ba13-a50ab024af9e

📥 Commits

Reviewing files that changed from the base of the PR and between c2f8433 and 152dbb2.

📒 Files selected for processing (1)

• src/core/legacy_aliases.rs

[coderabbitai]

Actionable comments posted: 0

[graycyrus]

@sanil-23 hey! the code looks good to me, but CI is still pending so i'll hold off on the formal approval until those checks are green — i'll come back and approve once they pass. let me know if you need anything.

For the record: the fix is clean. Scoping the comment-skip to line.starts_with("//") and the bare-identifier fallback to anything that doesn't lead with a quote character are both minimal and correct. The new test covers both the bare-key path and the comment-skip behavior, and CodeRabbit's request for explicit test coverage on bare identifiers is addressed. Nothing in scope here touches production code.