fix(workflows): evaluate parenthesised expressions

[NgoQuocViet2001]

Problem

Adding parentheses to a workflow expression silently inverts its result:

ctx = StepContext(inputs={"a": True, "b": False, "c": True})

evaluate_expression("{{ inputs.a or inputs.b and inputs.c }}",     ctx)  # True
evaluate_expression("{{ (inputs.a or inputs.b) and inputs.c }}",   ctx)  # False  ← same logic
evaluate_expression("{{ (inputs.n) }}",                            ctx)  # None   ← not 5

_find_top_level deliberately skips bracketed text so an operator inside a nested operand is not split on. Nothing then unwraps a group that spans the whole expression, so:

1. (a or b) and c splits at the top-level and
2. the left side (a or b) reaches the dot-path fallback
3. _resolve_dot_path looks up a key literally named (a or b), finds nothing, returns None

The or is never evaluated and the expression reads false. Parentheses are added precisely to make precedence explicit, so this fires on the expressions an author was being careful with.

Nothing catches it either: the syntax is valid, so condition_has_malformed_expression_block and condition_is_never_evaluated both pass it. An if: step gated on such a condition takes the wrong branch, and a while step never starts, with no diagnostic.

Fix

Unwrap a group that spans the whole expression, before the operator scans run. The check is quote-aware and requires the opening paren to close at the very last character, so it does not touch:

• (a) and (b) — the first group closes early, so the top-level and still splits
• (inputs.n) | default(9) — same reason; the filter still applies
• 'a(b' and ('(') — a paren inside a string literal is not a group

Scope

One helper and one early return in src/specify_cli/workflows/expressions.py. Unparenthesised expressions take exactly the path they did before.

Test plan

• Ran: pytest tests/test_workflows.py -k "parenthesised or indexing or Expressions" → 54 passed.
• Ran: pytest tests/test_workflows.py → 942 passed. The 20 failures are the TestWorkflowCliAlignment symlink cases, which fail identically on an unmodified checkout here (Windows, no symlink privilege).
• Checked: reverting only expressions.py fails the new test with assert False is True.

[Copilot]

🟡 Changes recommended

Parenthesis handling must also be mirrored in _unresolvable_term() with remediation coverage.

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

Pull request overview

Adds correct evaluation of fully parenthesized workflow expressions.

Changes:

• Unwraps complete parenthesized groups before evaluation.
• Adds grouping, nesting, filtering, and quote-awareness tests.

File summaries

File	Review
tests/test_workflows.py	Adds regression coverage for grouped expressions and edge cases.
src/specify_cli/workflows/expressions.py	Implements parenthesis unwrapping. The remediation parser remains inconsistent with evaluator behavior for grouped bare conditions (moderate; 2 votes).

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

---

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

[mnriem]

Please address Copilot feedback

[NgoQuocViet2001]

Addressed — the Copilot finding was correct, thanks.

_unresolvable_term() did not mirror the evaluator's unwrap, so a grouped operand reached the path check as literal text and the wrap correction was withheld from a condition that would have worked:

inputs.a or inputs.b                 -> Wrap the expression: "{{ ... }}".
(inputs.a or inputs.b) and inputs.c  -> No correction is offered because
                                        '(inputs.a or inputs.b)' is not a name
                                        the evaluator can resolve

The unwrap branch is mirrored now, placed to match where the evaluator does it. Covered both directions in test_condition_expression_block.py, since the unwrap must not become a blanket pass for anything parenthesised:

• (inputs.a or inputs.b) and inputs.c, (inputs.a), ((inputs.a)), (inputs.a) and (inputs.c) → correction offered
• (bogus or inputs.b) and inputs.c, (inputs.a or bogus) → still refused, naming bogus

pytest tests/unit/test_condition_expression_block.py → 341 passed; reverting only expressions.py fails the four new grouped cases.

[mnriem]

Thanks — parentheses silently inverting a result is exactly the kind of correctness bug worth fixing, and mirroring the unwrap in _unresolvable_term() (with both-direction tests so it's not a blanket pass) is the right call. Two things before merge: (1) please add the AI-disclosure per CONTRIBUTING — the body has none; (2) the Copilot review predates your fix, so I'll re-request it to confirm it's resolved. Green + disclosure and this is good to go.

[Copilot]

🟢 Approval recommended

The fix is focused, well-tested, and has no unresolved issues.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The implementation is focused, consistent across evaluation and validation, and adequately tested.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mnriem]

Green review, but this is part of the expressions.py cluster and isn't independently mergeable yet: (a) CI is stale (Sep 8, pre-current-main); (b) it overlaps #4416 in _unresolvable_term, so those two can't both merge as-is; and (c) per ntdatt812's experiment on #4460, once that refactor lands your _unresolvable_term hunk becomes unnecessary. Plan: land #4460 first, then rebase this to just the evaluator-side hunk (it'll shrink), re-run CI, add the disclosure — then merge. Holding on the cluster sequencing.