Add shared argument and paragraph fingerprint data models (8.1.3)#224
Merged
Conversation
…gerprint data models This new document outlines the design and implementation plan for roadmap item 8.1.3. It specifies the creation of shared argument and paragraph fingerprint data models in the common::rstest module, intended to serve future lint implementations. The execplan covers purpose, constraints, risks, progress checkpoints, and detailed decisions to ensure a stable and deterministic API. It includes comprehensive guidance on test coverage, API design, and documentation updates, forming the foundation for subsequent development steps. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
WalkthroughImplement shared rstest argument and paragraph fingerprint data models with deterministic normalisation, re-export types from common::rstest, add unit/proptest/BDD tests, add Markdown lint config, adjust Makefile PATH handling for tool discovery, and update ExecPlan/docs and the roadmap. ChangesArgument and Paragraph Fingerprint Data Models
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 18✅ Passed checks (18 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds a new ExecPlan markdown document that defines the execution plan, constraints, risks, milestones, and validation strategy for implementing shared argument and paragraph fingerprint data models in the whitaker-common rstest module; no code behavior changes are introduced. Class diagram for planned rstest fingerprint data modelsclassDiagram
class ArgAtom {
<<enum>>
+FixtureLocal name : String
+ConstLit text : String
+ConstPath def_path : String
+Unsupported
}
class ArgFingerprint {
+atoms : Vec~ArgAtom~
+new(atoms : Vec~ArgAtom~) ArgFingerprint
+atoms_ref() Vec~ArgAtom~
}
class LocalSlot {
+value : u16
+new(value : u16) LocalSlot
+get() u16
}
class CalleeShape {
<<enum>>
+DefPath value : String
+Unknown
}
class ExprShape {
<<enum>>
+Call callee : CalleeShape
+Call argc : usize
+MethodCall method : String
+MethodCall argc : usize
+Path
+Lit
+Other
}
class StmtShape {
<<enum>>
+Let init : ExprShape
+MutCall receiver : Option~LocalSlot~
+MutCall callee : CalleeShape
}
class ParagraphFingerprint {
+shapes : Vec~StmtShape~
+new(shapes : Vec~StmtShape~) ParagraphFingerprint
+shapes_ref() Vec~StmtShape~
}
ArgFingerprint --> ArgAtom : contains
ParagraphFingerprint --> StmtShape : contains
StmtShape --> ExprShape : uses
StmtShape --> LocalSlot : uses
StmtShape --> CalleeShape : uses
ExprShape --> CalleeShape : may reference
ParagraphFingerprint --> LocalSlot : uses via StmtShape
Flow diagram for 8.1.3 implementation milestones and validationflowchart TD
M1["Milestone 1
Lock contracts in tests
- Unit tests for fingerprints
- BDD scenarios"] --> M2["Milestone 2
Implement argument_fingerprint models"]
M2 --> M3["Milestone 3
Implement paragraph_fingerprint models
(with deterministic LocalSlot normalization)"]
M3 --> M4["Milestone 4
Re-export API from common::rstest and lib.rs
Add Rustdoc examples"]
M4 --> M5["Milestone 5
Update design docs and roadmap"]
M5 --> V["Validation
- cargo test -p whitaker-common rstest::
- cargo test rstest_fingerprint_behaviour
- cargo clippy -p whitaker-common --all-targets --all-features
- make fmt markdownlint nixie check-fmt lint test"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Introduce shared argument and paragraph fingerprint models in `whitaker-common` so later rstest hygiene lints can group repeated helper arguments and setup paragraphs without depending on compiler-private data. Add deterministic paragraph local-slot normalization, preserve unsupported argument atoms explicitly, and cover the public API with focused unit tests and rstest-bdd behaviour scenarios. Record the implementation decisions in the design notes and roadmap, and add legacy Markdown lint configuration so `make fmt` and `make markdownlint` share the same policy.
Prepend the standard user-local binary directories to the Makefile path so hook-driven validation can find `cargo`, `markdownlint-cli2`, and related local developer tools even when the hook environment does not inherit an interactive shell path. Record the post-turn hook finding in the 8.1.3 ExecPlan so future runs explain why the Makefile carries the explicit path setup.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`:
- Line 337: The API sketch declares LocalSlot with the wrong integer width;
update the type from LocalSlot(u16) to LocalSlot(u32) so the docs match the
implemented contract: locate the declaration of the LocalSlot struct in the
ExecPlan API sketch and replace the u16 payload with u32 (symbol: LocalSlot) to
remove the stale draft type and keep documentation consistent.
- Line 121: The doc line currently says "avoid `.expect()` in test code" which
conflicts with the repo's lint policy that allows `.expect()`/`.unwrap()` in
tests; update that sentence in the document to reflect policy by replacing it
with wording like "use `.expect()` messages deliberately in tests" (or similar),
e.g., change the phrase in the sentence that includes "prefer explicit
`Given`/`When`/`Then` transitions, and avoid `.expect()` in test code" to read
"prefer explicit `Given`/`When`/`Then` transitions, and use `.expect()` messages
deliberately in tests" so the guidance aligns with allow-expect-in-tests = true.
In `@Makefile`:
- Line 3: Replace the global PATH mutation (the line starting with "export PATH
:= $(HOME)/.cargo/bin:...") with a per-target tool path variable (e.g., declare
TOOL_PATH_SUFFIX := $(HOME)/.cargo/bin:$(HOME)/.bun/bin:$(HOME)/.local/bin) and
update targets that need those user-local tools (for example the "markdownlint"
target that invokes $(MDLINT)) to run with PATH="$(PATH):$(TOOL_PATH_SUFFIX)"
$(MDLINT) ... so the user-local bins are appended only for tooling targets
rather than prepending them globally.
🪄 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 Plus
Run ID: 9b0ab14d-75d3-4e4b-8441-44c71ce6b4b5
📒 Files selected for processing (13)
.markdownlint.jsonMakefilecommon/src/lib.rscommon/src/rstest/argument_fingerprint.rscommon/src/rstest/mod.rscommon/src/rstest/paragraph_fingerprint.rscommon/src/rstest/tests/fingerprint.rscommon/src/rstest/tests/mod.rscommon/tests/features/rstest_fingerprint.featurecommon/tests/rstest_fingerprint_behaviour.rsdocs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.mddocs/lints-for-rstest-fixtures-and-test-hygiene.mddocs/roadmap.md
Drop the global PATH prepend so default recipes inherit the caller's environment. Append `TOOL_PATH_SUFFIX` only for targets that invoke user-local binaries (tests, publish-check, fmt helpers, markdownlint, nixie, workflow tooling). Align the 8.1.3 ExecPlan sketch and BDD note with the implemented `LocalSlot(u32)` contract and test `expect()` policy. Co-authored-by: Cursor <cursoragent@cursor.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. |
This comment was marked as resolved.
This comment was marked as resolved.
Owner
Author
|
@coderabbitai re:
Please create a new GitHub issue for this |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 46
|
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. |
This comment was marked as resolved.
This comment was marked as resolved.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`:
- Around line 185-188: The sentence inaccurately says the Makefile "prepends"
tool directories to PATH; update the wording to reflect that the change is a
target-scoped (recipe-level) PATH extension rather than a global prepend. Edit
the passage to mention that each Makefile recipe temporarily extends PATH for
that target's commands (e.g., recipe-level or target-scoped PATH handling) so
tools like `cargo` and `markdownlint-cli2` resolve consistently during post-turn
hooks, and remove the implication of a global PATH modification.
- Around line 14-16: Update the approval sentence in
docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md: replace
the present-tense phrase "This plan was approved for implementation on
2026-05-01. Code changes for roadmap item 8.1.3 may proceed within the
tolerances below." with a past-tense completion statement (e.g., "This plan was
approved and implemented on 2026-05-01; roadmap item 8.1.3 is now complete.") so
the document matches the completed status noted at the top of the file.
In `@Makefile`:
- Line 6: Change the Makefile so TOOL_PATH_SUFFIX is defined with a recursive
assignment and shell-expanded HOME (use TOOL_PATH_SUFFIX =
$$HOME/.cargo/bin:$$HOME/.bun/bin:$$HOME/.local/bin instead of := with $(HOME)),
and update every export PATH usage to perform shell-time expansion by replacing
occurrences like export PATH="$(PATH):$(TOOL_PATH_SUFFIX)" and any "$(PATH)"
concatenations with export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; keep the
TOOL_PATH_SUFFIX reference unchanged elsewhere; apply this change to all
instances (the review lists the affected blocks) so Make no longer freezes
PATH/HOME at parse time on Windows CI.
🪄 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 Plus
Run ID: 5c8c9f2e-c009-489c-aa8c-e65b11c3c41d
📒 Files selected for processing (2)
Makefiledocs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md
Resolve review feedback by using shell-expanded user-local tool paths in Makefile recipes instead of freezing `HOME` or `PATH` at parse time. Update the 8.1.3 ExecPlan to describe the completed state and the recipe-level PATH handling accurately.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Add developer-guide coverage for the shared argument and paragraph fingerprint APIs introduced under `common::rstest`. Extend unit and property coverage for empty fingerprints, boundary atom values, `LocalSlot` roundtrips, and `ParagraphNormalizer` ordering and idempotence.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11421
✅
|
| Test | Line |
|---|---|
empty_argument_fingerprint_equals_another_empty |
98 |
empty_paragraph_fingerprint_equals_another_empty |
106 |
arg_atom_constructors_accept_empty_string |
114 |
arg_atom_constructors_accept_long_string |
124 |
LocalSlot::new(u32::MAX).index() boundary |
133 |
✅ Testing — Property / Proof
common/Cargo.tomlhasproptest = "1"under[dev-dependencies](line 33).common/src/rstest/tests/fingerprint_props.rsexists with fourproptest!property tests.common/src/rstest/tests/mod.rsdeclaresmod fingerprint_props;.
⚠️ Title check — one manual action required
The CLI command to rename the PR title is blocked by workflow restrictions. Update the title manually to:
Add shared argument and paragraph fingerprint data models (8.1.3)
leynos
changed the title
This was referenced May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Rationale and Scope
Plan and Milestones (high level)
Validation plan (for next steps)
Documentation impact
Risks and considerations
Reviewer notes
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/03f0d9aa-ec96-4949-852c-fd06b237fd72
Summary by Sourcery
Documentation: