Skip to content

refactor(guardrails): own the rule descriptor and hash, drop responsecache import#473

Merged
SantiagoDePolonia merged 1 commit into
mainfrom
refactor/guardrails-hash-ownership
Jul 4, 2026
Merged

refactor(guardrails): own the rule descriptor and hash, drop responsecache import#473
SantiagoDePolonia merged 1 commit into
mainfrom
refactor/guardrails-hash-ownership

Conversation

@SantiagoDePolonia

@SantiagoDePolonia SantiagoDePolonia commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a dependency inversion found in the architecture review (see docs/dev/2026-07-04_architecture-review.md on #472): GuardrailRuleDescriptor and ComputeGuardrailsHash were defined in responsecache although they are guardrail-domain concepts, which forced guardrails (a domain package) to import the cache layer.

  • Both move into internal/guardrails (hash.go); the descriptor is renamed RuleDescriptor to avoid guardrails.GuardrailRuleDescriptor stutter.
  • responsecache never used them in production code — the hash reaches the cache as an opaque context value (core.WithGuardrailsHash), which is unchanged. The cache's context helpers stay put.
  • Hash tests move with the code; workflows/admin test references updated.
  • guardrails no longer imports responsecache at all (verified via go list).

User-visible impact

None. Identical hash algorithm and wiring; pure ownership move.

Testing

Full go build ./... and go test ./... pass; pre-commit make test-race ran on the commit.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved guardrail processing so rule ordering no longer affects the resulting cache/hash behavior.
    • Made guardrail-related results more consistent when rule content or settings change.
    • Removed a dependency on an older internal guardrail descriptor path, reducing inconsistencies in guardrail registration.

…cache import

GuardrailRuleDescriptor and ComputeGuardrailsHash were defined in
responsecache although they are guardrail-domain concepts, forcing the
guardrails package to depend on the cache layer. Move them into guardrails
(descriptor renamed to RuleDescriptor to avoid package stutter); the cache
keeps consuming the hash as an opaque context value, which was already the
only way it received it. No behavior change; hash tests move with the code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

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: a1e648e2-c98f-460e-97cf-54324712eb09

📥 Commits

Reviewing files that changed from the base of the PR and between 5b885dd and 2e59ff9.

📒 Files selected for processing (8)
  • internal/admin/handler_workflows_test.go
  • internal/guardrails/definitions.go
  • internal/guardrails/hash.go
  • internal/guardrails/hash_test.go
  • internal/guardrails/registry.go
  • internal/responsecache/semantic.go
  • internal/responsecache/semantic_test.go
  • internal/workflows/compiler_test.go
💤 Files with no reviewable changes (2)
  • internal/responsecache/semantic.go
  • internal/responsecache/semantic_test.go

📝 Walkthrough

Walkthrough

Guardrail rule descriptor hashing is relocated from internal/responsecache to a new internal/guardrails/hash.go module, introducing RuleDescriptor and ComputeGuardrailsHash. All producers (definitions.go, registry.go) and consumers (admin and workflows tests) are updated to reference the guardrails package instead of responsecache, with the old type/function and their tests removed.

Changes

Guardrails Hash Relocation

Layer / File(s) Summary
New RuleDescriptor type and ComputeGuardrailsHash
internal/guardrails/hash.go, internal/guardrails/hash_test.go
Defines RuleDescriptor and ComputeGuardrailsHash, computing a deterministic SHA-256 hash from sorted per-rule seed strings; tests verify stability, order independence, and change-detection on content/order/mode.
Removal of old hashing API
internal/responsecache/semantic.go, internal/responsecache/semantic_test.go
Deletes the exported ComputeGuardrailsHash and GuardrailRuleDescriptor and their associated test suite.
Guardrails package internal usage update
internal/guardrails/definitions.go, internal/guardrails/registry.go
Switches buildDefinition, llmBasedAlteringDescriptor, registryEntry.descriptor, Register, and BuildPipeline to use the local RuleDescriptor and ComputeGuardrailsHash.
Downstream test callers updated
internal/admin/handler_workflows_test.go, internal/workflows/compiler_test.go
Updates test helpers to register guardrails with guardrails.RuleDescriptor and removes now-unused responsecache imports.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Registry Consumer
    participant Registry as guardrails.Registry
    participant Hash as guardrails.ComputeGuardrailsHash

    Caller->>Registry: Register(guardrail, RuleDescriptor)
    Registry->>Registry: store descriptor
    Caller->>Registry: BuildPipeline(references)
    Registry->>Registry: collect []RuleDescriptor
    Registry->>Hash: ComputeGuardrailsHash(descriptors)
    Hash-->>Registry: pipeline hash
    Registry-->>Caller: pipeline + hash
Loading

Poem

A hop, a skip, a type set free,
from responsecache to guardrails' tree.
Hash the rules, sort them tight,
SHA-256 shines bright at night. 🐇
No more borrowed burrows to keep—
now guardrails hash while I nibble and sleep. 🥕

🚥 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 refactor moving rule descriptor and hash ownership into guardrails and removing the responsecache dependency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/guardrails-hash-ownership

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.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/guardrails/definitions.go 40.00% 6 Missing ⚠️
internal/guardrails/hash.go 81.81% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Jul 4, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

Safe to merge with minimal risk.

The change is a focused ownership refactor. The hash algorithm was moved unchanged, tests moved with it, and call sites were updated consistently.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • Reviewed the guardrails refactor affected tests and confirmed internal tests pass and the affected-package test run exited with code 0, despite the tool wrapper timeout.
  • Re-ran the remaining affected tests (responsecache, workflows, and admin) to completion and verified the run exited with code 0.
  • Validated the import inversion results by inspecting the exact go list imports and confirming that guardrails imports do not include gomodel/internal/responsecache.
  • Validated the full go build across all packages completed with exit code 0.

View all artifacts

T-Rex Ran code and verified through T-Rex

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Definitions as guardrails definitions
participant Registry as guardrails Registry
participant Hash as guardrails ComputeGuardrailsHash
participant Core as core context
participant Cache as responsecache

Definitions->>Registry: Register(Guardrail, RuleDescriptor)
Registry->>Registry: BuildPipeline(steps)
Registry->>Hash: ComputeGuardrailsHash(descriptors)
Hash-->>Registry: guardrailsHash
Registry-->>Core: compiled policy carries hash
Core->>Cache: context includes opaque guardrails hash
Cache->>Cache: include hash in cache key
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Definitions as guardrails definitions
participant Registry as guardrails Registry
participant Hash as guardrails ComputeGuardrailsHash
participant Core as core context
participant Cache as responsecache

Definitions->>Registry: Register(Guardrail, RuleDescriptor)
Registry->>Registry: BuildPipeline(steps)
Registry->>Hash: ComputeGuardrailsHash(descriptors)
Hash-->>Registry: guardrailsHash
Registry-->>Core: compiled policy carries hash
Core->>Cache: context includes opaque guardrails hash
Cache->>Cache: include hash in cache key
Loading

Reviews (1): Last reviewed commit: "refactor(guardrails): own the rule descr..." | Re-trigger Greptile

@SantiagoDePolonia SantiagoDePolonia merged commit 0ed7417 into main Jul 4, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants