BREAKING: Rename 14 built-in functions for naming consistency#322
Conversation
BREAKING CHANGE: String operations gain `string_` prefix (strip → string_strip, upper → string_upper, lower → string_lower, replace → string_replace, split → string_split, join → string_join, contains → string_contains, starts_with → string_starts_with, ends_with → string_ends_with, index_of → string_index_of, char_code → string_char_code). Float predicates gain `float_` prefix (is_nan → float_is_nan, is_infinite → float_is_infinite). Conversion renamed (to_float → int_to_float). Document the four naming patterns (domain_verb, source_to_target, domain_is_predicate, prefix-less math universals) in spec §9.1.1, SKILL.md, vera/README.md, and CONTRIBUTING.md. Closes #288 Co-Authored-By: Claude <noreply@anthropic.invalid>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRenames numerous built-in functions to a consistent naming convention (e.g., Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Source as Source code
participant Parser as Parser / Checker
participant Env as Environment (builtins registry)
participant Codegen as Codegen / WASM translator
participant Runtime as WASM Runtime
Source->>Parser: parse fn call `string_strip(...)` or legacy `strip(...)`
Parser->>Env: resolve builtin name
Env-->>Parser: Found `string_strip` / NotFound for legacy name (emit warning)
Parser->>Codegen: emit call node with resolved builtin id
Codegen->>Codegen: map builtin id → WASM intrinsic (`string_strip`)
Codegen->>Runtime: compile/link WASM call
Runtime-->>Source: execute resulting intrinsic
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
spec/09-standard-library.md (1)
853-858:⚠️ Potential issue | 🟡 MinorSpecify a language for this fenced code block (MD040).
This fence is missing a language tag, which triggers the configured markdownlint check.
📝 Suggested lint fix
-``` +```vera public fn float_is_infinite(`@Float64` -> `@Bool`) requires(true) ensures(true) effects(pure)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@spec/09-standard-library.mdaround lines 853 - 858, The fenced code block
containing the function declaration for float_is_infinite lacks a language tag;
update the triple-backtick fence to include the language (e.g., "vera") so the
block reads ```vera before the lines for public fn float_is_infinite(@Float64->
`@Bool`) ... to satisfy the markdownlint MD040 rule and enable proper
highlighting.</details> </blockquote></details> <details> <summary>tests/test_checker.py (1)</summary><blockquote> `3006-3331`: _⚠️ Potential issue_ | _🟡 Minor_ **Lock in the breaking contract with explicit legacy-name failure tests.** These updates validate new names and type errors, but they do not explicitly assert that removed names (for example `to_float`, `is_nan`, `starts_with`) now fail resolution. Add negative tests so deprecated aliases cannot silently reappear. <details> <summary>🔎 Suggested test addition</summary> ```diff + `@pytest.mark.parametrize`("src, match", [ + (""" +private fn f(`@Int` -> `@Float64`) + requires(true) ensures(true) effects(pure) +{ to_float(`@Int.0`) } +""", "Unresolved"), + (""" +private fn f(`@Float64` -> `@Bool`) + requires(true) ensures(true) effects(pure) +{ is_nan(`@Float64.0`) } +""", "Unresolved"), + (""" +private fn f(`@String`, `@String` -> `@Bool`) + requires(true) ensures(true) effects(pure) +{ starts_with(`@String.0`, `@String.1`) } +""", "Unresolved"), + ]) + def test_removed_builtin_names_fail(self, src: str, match: str) -> None: + _check_err(src, match) ``` </details> As per coding guidelines, tests should flag “missing edge cases for new compiler features.” <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tests/test_checker.py` around lines 3006 - 3331, Add negative tests that assert deprecated builtin names no longer resolve: for each removed alias (e.g. to_float, is_nan, starts_with and any other legacy names), add _check_err cases mirroring the existing positive tests but calling the legacy name and expecting an appropriate error category (likely "name" or "type" depending on resolution semantics). Locate nearby tests in TestFloatConversions, TestFloatPredicateBuiltins, and TestStringSearchBuiltins (e.g. where int_to_float, float_is_nan, string_starts_with are tested) and add corresponding failure tests that call the old alias and assert resolution fails so deprecated names cannot silently reappear. ``` </details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In `@spec/09-standard-library.md`: - Line 12: The overview list of built-ins is missing two implemented string functions; update the summary sentence that enumerates built-ins to include string_strip and string_char_code so the documented list matches the standard library implementation (look for the sentence listing string_* functions in the spec and append these two names to that list). --- Outside diff comments: In `@spec/09-standard-library.md`: - Around line 853-858: The fenced code block containing the function declaration for float_is_infinite lacks a language tag; update the triple-backtick fence to include the language (e.g., "vera") so the block reads ```vera before the lines for public fn float_is_infinite(`@Float64` -> `@Bool`) ... to satisfy the markdownlint MD040 rule and enable proper highlighting. In `@tests/test_checker.py`: - Around line 3006-3331: Add negative tests that assert deprecated builtin names no longer resolve: for each removed alias (e.g. to_float, is_nan, starts_with and any other legacy names), add _check_err cases mirroring the existing positive tests but calling the legacy name and expecting an appropriate error category (likely "name" or "type" depending on resolution semantics). Locate nearby tests in TestFloatConversions, TestFloatPredicateBuiltins, and TestStringSearchBuiltins (e.g. where int_to_float, float_is_nan, string_starts_with are tested) and add corresponding failure tests that call the old alias and assert resolution fails so deprecated names cannot silently reappear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID:
bdb5ac74-48c2-4cc1-82e7-8be273c7592b⛔ Files ignored due to path filters (6)
docs/index.htmlis excluded by!docs/**examples/string_ops.verais excluded by!**/*.veratests/conformance/ch04_string_builtins.verais excluded by!**/*.veratests/conformance/ch09_string_search.verais excluded by!**/*.veratests/conformance/ch09_type_conversions.verais excluded by!**/*.veratests/conformance/ch10_float_predicates.verais excluded by!**/*.vera📒 Files selected for processing (19)
CHANGELOG.mdCONTRIBUTING.mdROADMAP.mdSKILL.mdTESTING.mdpyproject.tomlscripts/check_skill_examples.pyscripts/check_spec_examples.pyspec/04-expressions.mdspec/09-standard-library.mdtests/conformance/manifest.jsontests/test_checker.pytests/test_codegen.pyvera/README.mdvera/__init__.pyvera/codegen/modules.pyvera/environment.pyvera/wasm/calls.pyvera/wasm/inference.py
- Add 14 parametrized negative tests asserting all pre-#288 function names (to_float, is_nan, strip, etc.) produce "Unresolved" warnings - Add missing string_strip and string_char_code to spec §9 overview list - Update test counts in TESTING.md (2,404) and README.md Co-Authored-By: Claude <noreply@anthropic.invalid>
|
Addressed all actionable CodeRabbit findings in d9e2144:
|
|
@coderabbitai Addressed all actionable findings in d9e2144:
|
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
🧠 Learnings used |
Document the @coderabbitai reply convention and available commands so future sessions properly interact with the AI code review bot. Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/09-standard-library.md (1)
853-858:⚠️ Potential issue | 🟡 MinorAdd a language tag to this fenced code block.
This fence is missing a language identifier;
markdownlintflags it (MD040). Useverahere.Suggested patch
-``` +```vera public fn float_is_infinite(`@Float64` -> `@Bool`) requires(true) ensures(true) effects(pure)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@spec/09-standard-library.mdaround lines 853 - 858, The fenced code block
for the function declaration "public fn float_is_infinite(@Float64->@Bool)" is
missing a language tag; update the triple-backtick fence to include the language
identifier "vera" (i.e., changetovera) so the block is properly
recognized by markdownlint (MD040) while leaving the enclosed lines for
float_is_infinite unchanged.</details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In `@spec/09-standard-library.md`: - Line 35: The normative sentence "New functions must follow these patterns." in the spec (the rule that advises choosing a pattern or using `domain_verb`) uses lowercase RFC wording; change the lowercase "must" to the RFC 2119 uppercase token "MUST" so the sentence reads "New functions MUST follow these patterns." and ensure any other normative keywords in the surrounding paragraph follow RFC 2119 casing (e.g., MUST/SHOULD/MAY) consistently. In `@tests/test_checker.py`: - Around line 3173-3251: The test suite is missing a regression case for the removed legacy builtin name from_char_code; update the parameterized cases in test_removed_builtin_names_fail to include a snippet that calls from_char_code (e.g. a private fn using from_char_code(`@Int` -> `@String`) or equivalent) and expect the same "Unresolved" match, so the test verifies that from_char_code no longer resolves; modify the list passed to pytest.mark.parametrize in tests/test_checker.py (the test function test_removed_builtin_names_fail and helper _warnings are the relevant symbols) to add this case. --- Outside diff comments: In `@spec/09-standard-library.md`: - Around line 853-858: The fenced code block for the function declaration "public fn float_is_infinite(`@Float64` -> `@Bool`)" is missing a language tag; update the triple-backtick fence to include the language identifier "vera" (i.e., change ``` to ```vera) so the block is properly recognized by markdownlint (MD040) while leaving the enclosed lines for float_is_infinite unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID:
954d31ba-093a-4211-8486-7b113b9865f7📒 Files selected for processing (4)
README.mdTESTING.mdspec/09-standard-library.mdtests/test_checker.py
- Use RFC 2119 MUST casing in spec §9.1.1 naming convention rule - Add from_char_code to legacy name regression tests (15 total) - Update doc counts to 2,405 tests Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 588: Replace the outdated release version literal "v0.0.85" in README.md
with the current release version "0.0.92" (the string identified as v0.0.85 in
the file) so the README matches the PR/release metadata referencing 0.0.92;
ensure any other occurrences of the old version string in the same file are
updated consistently.
In `@spec/09-standard-library.md`:
- Line 12: The spec currently mentions the builtins string_strip and
string_char_code but lacks formal §9.6.13 entries; add normative specification
entries for both functions including their signatures, short descriptions,
parameter/return types, and at least one runnable example each (mirror the
existing string_from_char_code round-trip example to show string_char_code ->
string_from_char_code behavior), and update any cross-references; specifically
implement signature/example blocks for string_strip and string_char_code so the
standard library list and examples are consistent and complete.
In `@TESTING.md`:
- Line 9: Update the stale hard-coded test counts in TESTING.md so the document
is the single source of truth: search for the mentions of test_conformance.py
and change the parametrised count from 260 to 275, update the check_examples.py
example count from 23 to 25, and reconcile any related summary numbers (the
table row showing total Tests and any other occurrences) so all figures match
the actual codebase counts; verify by running the test-discovery scripts or
reading the parametrization in test_conformance.py and the examples list used by
check_examples.py to ensure consistency.
In `@tests/test_checker.py`:
- Around line 2660-2665: Add missing negative tests for the renamed built-ins:
create failing test cases (e.g., test_string_char_code_bad and
test_string_strip_bad) that mirror the existing happy-path tests
(test_string_char_code_ok and the string_strip success case) but pass incorrect
argument types to string_char_code and string_strip so the type checker rejects
them; follow the existing test patterns in this file (use the same helper used
for failures in other built-in tests) and add the corresponding assertions near
the current success tests (also apply the same change around the other
occurrence at lines ~2822-2827).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5899789e-6925-4765-a392-d7653e9410c0
📒 Files selected for processing (4)
README.mdTESTING.mdspec/09-standard-library.mdtests/test_checker.py
| | Metric | Value | | ||
| |--------|-------| | ||
| | **Tests** | 2,390 across 24 files (~24,500 lines of test code) | | ||
| | **Tests** | 2,405 across 24 files (~24,600 lines of test code) | |
There was a problem hiding this comment.
Refresh the remaining hard-coded counts in this file.
These totals were updated, but the same document still has stale figures further down: Line 146 says test_conformance.py parametrises 260 tests, and Line 304 says check_examples.py validates 23 examples. With the overview now reporting 275 conformance tests and 25 examples, TESTING.md is no longer internally consistent as the “single source of truth”.
As per coding guidelines, Markdown files should be reviewed for factual accuracy against the codebase.
Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TESTING.md` at line 9, Update the stale hard-coded test counts in TESTING.md
so the document is the single source of truth: search for the mentions of
test_conformance.py and change the parametrised count from 260 to 275, update
the check_examples.py example count from 23 to 25, and reconcile any related
summary numbers (the table row showing total Tests and any other occurrences) so
all figures match the actual codebase counts; verify by running the
test-discovery scripts or reading the parametrization in test_conformance.py and
the examples list used by check_examples.py to ensure consistency.
There was a problem hiding this comment.
@coderabbitai Fixed in ceaa73b — updated the stale conformance test count from 260 to 275 in TESTING.md.
There was a problem hiding this comment.
@aallan, thanks — the conformance test count fix (260 → 275) is confirmed. However, there's still one remaining inconsistency I flagged in the original comment: the Validation Scripts table lists check_examples.py as validating 23 examples, while the Pre-commit Hooks table (and the Overview row) correctly say 25. Was that also addressed in ceaa73b, or is it still outstanding?
🧠 Learnings used
Learnt from: CR
Repo: aallan/vera PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T17:16:46.789Z
Learning: Applies to **/*.vera : All Vera code must be formatted to canonical form using `vera fmt --check` to verify; use `vera fmt --write` to format in place
Learnt from: CR
Repo: aallan/vera PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T17:16:46.789Z
Learning: Applies to spec/**/*.md,README.md,SKILL.md,FAQ.md,**/*.html : When referencing Vera code in documentation (spec, README, SKILL.md, FAQ, HTML files), verify all code blocks parse and type-check using corresponding validation scripts (`check_spec_examples.py`, `check_readme_examples.py`, `check_skill_examples.py`, `check_faq_examples.py`, `check_html_examples.py`)
Learnt from: CR
Repo: aallan/vera PR: 0
File: FAQ.md:0-0
Timestamp: 2026-03-19T17:44:18.385Z
Learning: Run `vera test` to automatically generate test inputs from function contracts and validate postconditions without writing manual test cases
Learnt from: CR
Repo: aallan/vera PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T17:16:46.789Z
Learning: Applies to examples/**/*.vera : All Vera examples in `examples/` directory must pass both `vera check` and `vera verify` validation
Learnt from: CR
Repo: aallan/vera PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-19T17:17:03.012Z
Learning: All examples in examples/ must pass vera check and vera verify
Learnt from: CR
Repo: aallan/vera PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-19T17:44:00.853Z
Learning: Applies to spec/**/*.md : Ensure all code examples in specification documents are valid Vera and use canonical formatting rules from Chapter 1
Learnt from: CR
Repo: aallan/vera PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T17:16:46.789Z
Learning: Applies to tests/conformance/ch[0-9][0-9]_*.vera : All 55 conformance test programs in `tests/conformance/` must be named `chNN_feature.vera` with a header comment indicating the spec chapter and features tested, and must pass their declared conformance level
Learnt from: CR
Repo: aallan/vera PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-19T17:44:00.853Z
Learning: Add or update tests as appropriate when making code changes
Learnt from: CR
Repo: aallan/vera PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-19T17:44:00.853Z
Learning: Update relevant documentation and tests when making changes
Learnt from: CR
Repo: aallan/vera PR: 0
File: SKILL.md:0-0
Timestamp: 2026-03-19T17:43:44.841Z
Learning: Applies to **/*.vera : Use string functions: `string_length`, `string_concat`, `string_slice`, `char_code`, `from_char_code`, `string_repeat`, `parse_*` (nat/int/float64/bool), `to_string`, `to_upper`, `to_lower`, `replace`, `split`, `join`, `starts_with`, `ends_with`, `string_contains`, `index_of` for string manipulation
- Add formal spec §9.6.13 entries for string_strip and string_char_code with signatures, descriptions, and examples - Add negative type-checker tests for string_char_code and string_strip (wrong argument types) - Fix stale README version (v0.0.85 → v0.0.92) - Fix stale TESTING.md conformance count (260 → 275) - Update allowlist line numbers after spec insertion - Update doc counts to 2,407 tests Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
TESTING.md (1)
9-9:⚠️ Potential issue | 🟡 MinorRefresh the remaining hard-coded totals in this file.
These count updates are still incomplete: line 304 says
check_examples.pyvalidates 23 examples, while the overview, command list, and hook list now all say 25. The overview’s contract-verification row is also out of sync with the detailed section at lines 202-204, so the supposed single source of truth is still internally inconsistent.As per coding guidelines, Markdown files should be reviewed for factual accuracy against the codebase.
Also applies to: 146-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TESTING.md` at line 9, Update the hard-coded totals in TESTING.md so they are consistent with the actual counts: change the check_examples.py example count from 23 to 25 (referenced in the check_examples.py line), update the overview's contract-verification row to match the detailed contract-verification section (the detailed section at lines ~202-204), and refresh the overall tests total and any other outdated hard-coded counts shown in the overview, command list, and hook list (also the occurrences flagged around line 146 and line 304) so every summary and detailed entry matches the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 588: The README claim that the "91% code coverage [is] enforced by
pre-commit hooks and CI" is inaccurate; either remove the "pre-commit hooks"
phrase from that sentence to match TESTING.md or add a real pre-commit hook that
enforces the same coverage gate. If choosing to add the hook, create a
pre-commit entry that runs the test command with --cov and fails if coverage <
91% (matching the CI threshold) and document this new hook in TESTING.md; if
choosing to remove, update the README sentence to only reference CI/coverage as
documented in TESTING.md.
In `@spec/09-standard-library.md`:
- Around line 18-35: The naming rules claim only math universals are
prefix-less, but §9.6.11 exports nan() and infinity() as prefix-less float
constants—update the spec to be consistent by either (A) adding an explicit
exception in §9.1.1 for float constants (e.g., allow prefix-less nan() and
infinity() as well as document float_is_nan/float_is_infinite as predicate
alternatives) or (B) changing §9.6.11 to export them with the float_ prefix
(float_nan(), float_infinity()) and remove the prefix-less forms; reference
sections 9.1.1 and 9.6.11 and the identifiers nan() and infinity() when making
the edit.
In `@tests/test_checker.py`:
- Around line 3264-3269: The test test_removed_builtin_names_fail currently only
checks that a matching Unresolved warning appears via _warnings(src) but doesn't
ensure no checker errors are produced; update the test to also call _check(src)
and assert it returns no errors (mirroring test_unresolved_function_warning) so
legacy names remain warning-only—locate test_removed_builtin_names_fail and add
an assertion that _check(src) yields an empty error list or equivalent empty
result.
---
Duplicate comments:
In `@TESTING.md`:
- Line 9: Update the hard-coded totals in TESTING.md so they are consistent with
the actual counts: change the check_examples.py example count from 23 to 25
(referenced in the check_examples.py line), update the overview's
contract-verification row to match the detailed contract-verification section
(the detailed section at lines ~202-204), and refresh the overall tests total
and any other outdated hard-coded counts shown in the overview, command list,
and hook list (also the occurrences flagged around line 146 and line 304) so
every summary and detailed entry matches the single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f22d56a0-a723-4c5e-ad07-359f4c49ab6e
📒 Files selected for processing (5)
README.mdTESTING.mdscripts/check_spec_examples.pyspec/09-standard-library.mdtests/test_checker.py
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
…st tightening - Fix README coverage claim: pre-commit runs tests but does not gate on coverage percentage, so remove pre-commit hooks from the claim - Widen spec 9.1.1 rule 4 to include float constants (nan, infinity) as prefix-less exceptions alongside math universals - Update SKILL.md naming convention table and summary to match - Tighten test_removed_builtin_names_fail to also assert no errors (warning-only behaviour), not just that a warning exists Co-Authored-By: Claude <noreply@anthropic.invalid>
Summary
domain_verbconvention: string operations gainstring_prefix (strip→string_strip,upper→string_upper, etc.), float predicates gainfloat_prefix (is_nan→float_is_nan,is_infinite→float_is_infinite), andto_float→int_to_floatdomain_verb,source_to_target,domain_is_predicate, prefix-less math universals)Breaking changes
All 14 old names will fail to resolve. The full rename list:
stripstring_stripupperstring_upperlowerstring_lowerreplacestring_replacesplitstring_splitjoinstring_joincontainsstring_containsstarts_withstring_starts_withends_withstring_ends_withindex_ofstring_index_ofchar_codestring_char_codeis_nanfloat_is_nanis_infinitefloat_is_infiniteto_floatint_to_floatTest plan
Closes #288
🤖 Generated with Claude Code
Summary by CodeRabbit
Changed
Documentation
Tests
Chores