Repository navigation
fix: print negated exponents without parentheses - #1
Conversation
Negation now shares the precedence of exponentiation, so the printer leaves -1 bare in 2 ^ -1, matching the grammar. The parser accepts exactly the same inputs as before, because no operator had precedence 3. Also: a lone : is a blank line rather than an unknown command; withSemiring is renamed withCarrier; the interpreter maps a missing result to DivisionByZero in one place; tests read the magnitude bound from the default configuration; the README states how piped input reports errors.
ECC Tools / Security EvidenceCommit: Security evidence gate passed (success) No security-sensitive scanner-evidence gap detected. Mode: enforce Scanned 9 changed file(s). No missing scanner-evidence signal was detected. Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / PR Risk TaxonomyCommit: PR taxonomy clear (success) Scanned 9 changed file(s). No taxonomy bucket signals were detected. Scanned 9 changed file(s). No PR taxonomy bucket signals were detected. Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
ECC Tools / Reference Set ReadinessCommit: Reference set readiness gaps detected (neutral) Reference evidence present for 0/7 areas (0%) across 9 changed file(s). This check is based on files changed in this PR. Repository-level readiness is still reported by
Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / Hosted Promotion ReadinessCommit: Hosted promotion readiness passed (success) No hosted promotion evidence gaps detected across 9 changed file(s); 0 corpus scenarios had matching evidence. This check compares PR file changes against the evaluator/RAG promotion corpus in No evaluator corpus scenarios matched this PR. Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c8cf2147-eabc-4c7b-b734-6a40802d9c04) |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new empty-name guard silently ignores directives containing non-whitespace text after a spaced colon.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Updates expression printing, REPL handling, error conversion, and test configuration.
Changes:
- Prints negated exponents without redundant parentheses.
- Handles a lone
:as blank input and documents batch-mode errors. - Consolidates helper logic and removes duplicated test constants.
| File | Description |
|---|---|
src/Calculator/Syntax.hs |
Aligns negation and exponentiation precedence. |
src/Calculator/Repl.hs |
Handles empty directive names. |
src/Calculator/Interpreter.hs |
Centralizes division-by-zero conversion. |
src/Calculator/Validation.hs |
Renames the carrier-evidence helper. |
test/Calculator/ParserSpec.hs |
Expands canonical printer coverage. |
test/Calculator/ReplSpec.hs |
Tests lone-colon handling. |
test/Calculator/InterpreterSpec.hs |
Uses the configured magnitude bound. |
test/Calculator/RenderSpec.hs |
Uses default configuration and typed errors. |
README.md |
Documents batch behavior and updates test count. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Raises negationPrecedence so printSyntax now prints forms like 3 * -2, 1 - -2 and 2 ^ -1 without parentheses, and treats a bare : as a Blank line rather than a directive. Folds the interpreter's division-by-zero handling into one orDivisionByZero helper shared by Divide and IntegerPower, and renames withSemiring to withCarrier. Tests now take the magnitude bound from defaultConfiguration instead of hard-coding it, and the README documents that piped input prints each line's error in place of its result, keeps going, and exits 0.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 123 functions depend on the 104 functions this change touches.
Health — this change adds coupling hotspots:
- new:
step()— 4 callers, 6 callees - new:
present()— 3 callers, 5 callees - new:
diagnose()— 3 callers, 4 callees - new:
spec()— 0 callers, 7 callees - new:
go()— 0 callers, 6 callees - new:
spec()— 0 callers, 6 callees - new:
spec()— 0 callers, 6 callees
Verification — 123 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 123 function(s) in the blast radius were not formally verified this run
· 7 more finding(s) on lines outside this diff (see the check run).
Fixes found in review.
2 ^ -1printed as2 ^ (-1), and:typeshowed the extra parentheses. Negation now shares the precedence of exponentiation. The parser accepts the same inputs as before, since no operator had precedence 3.:in the interactive session is treated as a blank line instead of an unknown command.withSemiringis renamedwithCarrier; it supplies bothSemiringandHeight.DivisionByZeroin one helper instead of two call sites.defaultConfigurationinstead of repeating the literal.Tests: 204 examples, 0 failures. The three new printer tests fail against the previous precedence.
Note
Low Risk
Precedence and printing tweaks with small REPL and refactor changes; behavior is covered by expanded tests and matches documented Python-style rules.
Overview
Printing and precedence: Unary negation now uses the same precedence level as exponentiation (
negationPrecedence3 → 4), so canonical output and:typeshow forms like2 ^ -1instead of2 ^ (-1). Parser behavior is unchanged because no other operator used precedence 3.REPL and docs: A line that is only
:is parsed as a blank line, not an unknown command. The README documents piped/batch mode: errors print in place of results, later lines still run, exit status stays 0, and the example count is updated to 204.Refactors: Validation renames
withSemiringtowithCarrier(still suppliesSemiringandHeight). The interpreter consolidates division-by-zero handling intoorDivisionByZeroinstead ofrefuseZeroat divide and integer-power sites.Tests: Parser specs add minimal-print and redundant-parenthesis cases (including negated exponents). Interpreter and render specs take the magnitude bound from
defaultConfiguration; REPL specs cover:as blank.Reviewed by Cursor Bugbot for commit a8bbf41. Configure here.