Skip to content

test: assert authoringMetaV2 word names and descriptions - #130

Merged
thedavidmeister merged 8 commits into
mainfrom
fix/issue-84-authoring-meta-assertion
Jul 28, 2026
Merged

test: assert authoringMetaV2 word names and descriptions#130
thedavidmeister merged 8 commits into
mainfrom
fix/issue-84-authoring-meta-assertion

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

testSubParserParseMeta verified that authoringMetaV2() produces bytes that hash correctly into the parse meta, but never asserted the actual word names or description strings. Any mutation to a word name or description text would silently pass.

Adds testAuthoringMetaV2Content which decodes the authoring meta and pins:

  • Array length == 3
  • Each word field (ftso-current-price-usd, ftso-current-price-pair, sflr-exchange-rate)
  • Full description string for each word

Mutation-validated: flipping "ftso-current-price-usd""ftso-current-price-us" in the source produces a bytes32 assertion failure.

Closes #84

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

  • Tests
    • Strengthened validation for decoded authoring metadata by asserting exact-match descriptions for three key entries, alongside existing checks for entry count and expected word values.

Closes #84

Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jun 16, 2026
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1d250d32-dae3-4ae6-92d8-313342a19a00

📥 Commits

Reviewing files that changed from the base of the PR and between 3a21eed and b11da46.

📒 Files selected for processing (1)
  • test/src/concrete/FlareFtsoWords.pointers.t.sol

Walkthrough

The existing authoringMetaV2() content test now validates the exact description strings for the FTSO USD price, FTSO price pair, and sFLR exchange rate entries instead of only checking that descriptions are non-empty.

Changes

Authoring metadata content test

Layer / File(s) Summary
Strengthen authoringMetaV2 description assertions
test/src/concrete/FlareFtsoWords.pointers.t.sol
Replaces non-empty checks with exact expected-string comparisons for three decoded metadata entries; array length and word assertions remain unchanged.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

🚥 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 change: asserting authoringMetaV2 word names and descriptions in tests.
Linked Issues check ✅ Passed The PR directly addresses #84 by adding exact description assertions and verifying the word-name/index mapping.
Out of Scope Changes check ✅ Passed The change stays within scope, limited to strengthening the existing authoringMetaContent test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-84-authoring-meta-assertion

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

Producer note: backing off per protocol — this branch already carries a merge(main): resolve conflicts [merge-update] commit but main has moved again and the PR is CONFLICTING once more. Single conflicting file: test/src/concrete/FlareFtsoWords.pointers.t.sol. This PR adds testAuthoringMetaV2Content (pins exact word names + full description strings); main has since landed testAuthoringMetaContent (same words, but only asserts descriptions are non-empty) plus testOpcodePointersLength / testBytecodeHashMatchesDeployedCode at the same insertion point. A mechanical union (keep both, no name clash) is possible, but main's test partially supersedes this PR's — needs a human call: keep both tests, or rework to strengthen main's testAuthoringMetaContent in place with the full-description pins.

@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

🤖 ai:producer
Producer note: state change — sibling PR #173 (pointer/authoring tests for #81/#84) has MERGED into main, but issue #84 is still OPEN. Producer rework queued: diff this branch's authoringMetaV2 word-name/description assertions against what #173 landed, keep only the remainder #84 still needs (or flag close-candidate if #173 fully covers it), against current main.

@coderabbitai coderabbitai Bot 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.

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 `@test/src/concrete/FlareFtsoWords.pointers.t.sol`:
- Around line 54-71: The tests `testAuthoringMetaV2Content` and
`testAuthoringMetaContent` overlap by decoding
`LibFlareFtsoSubParser.authoringMetaV2()` and asserting the same `word`/length
coverage for the same three entries. Consolidate this into a single test by
removing `testAuthoringMetaV2Content` and strengthening
`testAuthoringMetaContent` to assert the full description strings for each
`AuthoringMetaV2` entry, keeping the existing unique `word` checks in that one
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: ASSERTIVE

Plan: Pro

Run ID: 40d77796-75b3-4aac-83db-d097b47d396c

📥 Commits

Reviewing files that changed from the base of the PR and between a91ffe1 and 3a21eed.

📒 Files selected for processing (1)
  • test/src/concrete/FlareFtsoWords.pointers.t.sol

Comment thread test/src/concrete/FlareFtsoWords.pointers.t.sol Outdated
@thedavidmeister thedavidmeister added the ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) label Jul 6, 2026
@thedavidmeister

thedavidmeister commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 ai:vetter
Reviewed 3a21eed: reject — missing/false QA evidence: body lacks QA-GUIDE section-8 block. inline mutation claim exists but no
cost 138 — test-only authoring-meta content pin

@thedavidmeister thedavidmeister added the human:needs-work Human reviewer: needs rework label Jul 9, 2026
@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

Rework note (human): now stale after #136 merged. Line 24 asserts the OLD sflr-exchange-rate description "Returns the current exchange rate of FLR to SFLR.", which #136 replaced with "Returns the current sFLR per FLR exchange rate self-reported by the Sceptre staked FLR contract, i.e. how many sFLR shares correspond to 1 FLR. A value less than 1 means 1 FLR yields fewer than 1 sFLR share. Accepts 0 inputs.". Rebase on main and update that assertion to the new string (the rest of the name/description pins look fine). Otherwise the test fails against current main.

thedavidmeister and others added 3 commits July 10, 2026 09:28
…work)

Merges main (the #136 description rewrite landed after this branch) and
pins authoringMeta[2].description to the source string in
LibFlareFtsoSubParser.sol.

Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister thedavidmeister added ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) and removed ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) human:needs-work Human reviewer: needs rework labels Jul 16, 2026
@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

🤖 ai:vetter
Reviewed fc9ef5b: reject — closes #84 — missing QA-GUIDE §8 evidence block; also duplicates main's existing testAuthoringMetaContent (length + all 3 word names already pinned there, and its hardcoded length==3 regresses the constant-tied assertion): the residual #84 gap is description CONTENT only — extend the existing test's non-empty checks to full strings instead of adding a near-twin
cost 143 — test overlap analysis vs merged main

…t, drop near-twin

Per the vetter reject at fc9ef5b: the added testAuthoringMetaV2Content
duplicated main's testAuthoringMetaContent (names + constant-tied length
already pinned there) while regressing the length pin to a hardcoded 3.
Extend the existing test's non-empty description checks to full exact
strings instead — the residual #84 gap was description content only.

Mutation-validated: mutating the sflr-exchange-rate description in
LibFlareFtsoSubParser.sol fails testAuthoringMetaContent; restored passes.

Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

🤖 ai:producer
Producer note: reworked per the vetter reject at fc9ef5b — dropped the near-twin testAuthoringMetaV2Content (and its hardcoded length==3) and extended main's testAuthoringMetaContent full-string description pins in place of the non-empty checks (commit b11da46). Mutation evidence: mutating the sflr-exchange-rate description in LibFlareFtsoSubParser.sol fails testAuthoringMetaContent (string mismatch); restored → 8/8 suite green. The reject's other ground — QA §8 evidence block in the PR BODY — is unservable by this routine (no sanctioned body-edit path); tracked in rainlanguage/issue-pr-cron#51.

@thedavidmeister thedavidmeister added ai:ready AI vetter: passes review, ready for human decision and removed ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) labels Jul 18, 2026
@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

🤖 ai:vetter
Reviewed b11da46: ready — closes #84 — rework resolves both prior reject grounds: near-twin dropped, main's testAuthoringMetaContent extended in place with full-string description pins (constant-tied length kept; strings match LibFlareFtsoSubParser.sol byte-for-byte); QA mutation evidence in trusted producer comment at head (body-edit unservable, tracked issue-pr-cron#51); descriptions' input-count claims human-validated in #199's reject note; full-string pins subsume #199's planned contains-checks
cost 176 — test-only string pins, cross-PR context

@thedavidmeister

Copy link
Copy Markdown
Collaborator Author

Reviewed b11da46: approve

I checked the one thing that would have made this PR reproduce the defect it fixes: whether the expected strings are literals or read back from the same source the code uses. They are literals.

assertEq(
    m[SUB_PARSER_WORD_FTSO_CURRENT_PRICE_USD].description,
    "Returns the current USD price of the given token according to the FTSO. Accepts 2 inputs, ..."
);

The full text is written out in the test file, not referenced from the constant. So it is an independent oracle — change the wording in the source and this fails. Had it read the same constant, it would have agreed with the code by construction, one layer up from the original problem.

What it replaces is the non-discriminating shape, which makes this a real strengthening rather than an addition:

-        assertTrue(bytes(m[SUB_PARSER_WORD_FTSO_CURRENT_PRICE_USD].description).length > 0);

A length-greater-than-zero check passes for any non-empty string, so every description could have been replaced with "x" and the suite stayed green. Combined with the pre-existing situation described in #84testSubParserParseMeta verifying that authoringMetaV2() hashes correctly into the parse meta, where the expected hash derives from the same source — the word text had no assertion anywhere that a mutation could fail.

The word names were already pinned as bytes32(...) literals; this brings the descriptions to the same standard.

Worth stating so it is not a surprise later: these descriptions are now change-detecting, so a deliberate rewording requires updating this test. That is the intended cost of a pin, and the duplication between source and test is the oracle — not something to factor out.

Verified independently: ahead_by=8 behind_by=0 against main; copy-artifacts, rainix-sol legal / static / test all SUCCESS. CodeRabbit reads SUCCESS — treat that as the rate-limited stub unless a review body says otherwise.

@thedavidmeister
thedavidmeister merged commit b634aad into main Jul 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:ready AI vetter: passes review, ready for human decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[F33] [LOW] authoringMetaV2 description/name string content is never asserted by any test

1 participant