Add rule engine Phases 6-7: docs, protocol, factory, release hardening#132
Add rule engine Phases 6-7: docs, protocol, factory, release hardening#132
Conversation
Completes the analysis rule engine (WI-16 and WI-17):
- RuleEngineProtocol (typing.Protocol) — structural contract for
alternative engine implementations
- create_engine() factory — reads [rules].engine from INI, currently
supports "hy" only, clear error for unknown engines
- 4 Diataxis docs:
- tutorials/your-first-rule.md — end-to-end walkthrough
- how-to/authoring-rules.md — 7 common patterns (promotion,
refutation, blocking, AI ceiling, staleness, trust gate, annotation)
- reference/rule-engine-spec.md — THE SPEC: rule format grammar,
26 helpers, evaluation model, audit model, policy parameters
- explanation/rule-engine.md — architecture, Hy rationale, two-engine
coexistence, advisor pattern, evidence resolution, AI ceiling design
- CHANGELOG, CLAUDE.md, README.md updated with rule engine feature
- All 17 work items from the implementation spec complete
https://claude.ai/code/session_01H5UbjsuiiGya5n1eUCxoaR
There was a problem hiding this comment.
Pull request overview
Adds the final “release hardening” layer around the Hy-based analysis rule engine by formalizing an engine interface, adding a config-driven engine factory, and publishing Diataxis documentation plus release notes/feature surfacing across top-level docs.
Changes:
- Introduces
RuleEngineProtocol(typing.Protocol) to define the structural contract for rule engines. - Adds
create_engine()factory to construct a rule engine from[rules]INI config (currently supportshyonly). - Adds full Diataxis documentation set (tutorial/how-to/reference/explanation) and updates README/CLAUDE/CHANGELOG to surface the feature.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gnat/analysis/rules/protocol.py | Adds a structural typing protocol for rule engine implementations. |
| gnat/analysis/rules/factory.py | Adds INI-driven engine factory and unknown-engine validation. |
| docs/tutorials/your-first-rule.md | New end-to-end tutorial for authoring and running a first rule. |
| docs/how-to/authoring-rules.md | New recipe-style guide with common rule patterns and priority guidance. |
| docs/reference/rule-engine-spec.md | New authoritative spec (rule grammar, helpers, evaluation/audit model, policy keys). |
| docs/explanation/rule-engine.md | New design/architecture explanation and rationale for Hy + orchestration model. |
| README.md | Surfaces the Rule Engine feature and the gnat[rules] extra. |
| CLAUDE.md | Documents the new analysis/rules package and gnat[rules] extra. |
| CHANGELOG.md | Adds Unreleased entry describing the analysis rule engine feature set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (defrule <rule-name> | ||
| :description <string> ;; optional | ||
| :phase <string> ;; required: "open", "supported", "refuted", "inconclusive" | ||
| :target-status <string> ;; optional: informational only | ||
| :priority <integer> ;; optional, default 50 | ||
| :tags [<string> ...] ;; optional, default [] | ||
| :when (fn [h ctx] <body>) ;; required: predicate returning truthy/falsy | ||
| :then (fn [h ctx] <body>)) ;; required: returns a Decision | ||
| ``` |
There was a problem hiding this comment.
This spec is internally inconsistent about :phase: the rule format section says it’s required and a string limited to specific status values, but later the “Phase gates” section says phase = None matches all statuses. Please clarify in one place (e.g., :phase is required but may be a status string or None).
| ### Phase gates | ||
|
|
||
| A rule only evaluates if `hypothesis.status.value == rule.phase`. Rules | ||
| with `phase = None` match all statuses. | ||
|
|
There was a problem hiding this comment.
The “Phase gates” section says phase = None matches all statuses, but earlier the grammar describes :phase as a required status string. Update one or both sections so the allowed values/behavior match the actual implementation and don’t confuse rule authors.
|
|
||
| engine_name = "hy" | ||
| if hasattr(config, "get") and hasattr(config, "has_section") and config.has_section("rules"): | ||
| engine_name = config.get("rules", "engine", fallback="hy") |
There was a problem hiding this comment.
engine_name is read from config verbatim; values like "Hy"/" hy " will be rejected even though they’re semantically the same engine. Consider normalizing with .strip().lower() (similar to other config factories) before validating against _SUPPORTED_ENGINES.
| engine_name = config.get("rules", "engine", fallback="hy") | |
| engine_name = config.get("rules", "engine", fallback="hy").strip().lower() |
| if policy is None: | ||
| policy = RuleEnginePolicy.from_ini(config) | ||
|
|
||
| engine_name = "hy" | ||
| if hasattr(config, "get") and hasattr(config, "has_section") and config.has_section("rules"): | ||
| engine_name = config.get("rules", "engine", fallback="hy") |
There was a problem hiding this comment.
config is annotated as Any and guarded with hasattr, but RuleEnginePolicy.from_ini(config) requires a configparser.ConfigParser and will fail for other types (e.g., GNATConfig). Either narrow the parameter type to ConfigParser and drop the hasattr checks, or explicitly support GNATConfig by pulling config.parser before calling from_ini/get.
| if policy is None: | |
| policy = RuleEnginePolicy.from_ini(config) | |
| engine_name = "hy" | |
| if hasattr(config, "get") and hasattr(config, "has_section") and config.has_section("rules"): | |
| engine_name = config.get("rules", "engine", fallback="hy") | |
| ini_config = getattr(config, "parser", config) | |
| if policy is None: | |
| policy = RuleEnginePolicy.from_ini(ini_config) | |
| engine_name = "hy" | |
| if ( | |
| hasattr(ini_config, "get") | |
| and hasattr(ini_config, "has_section") | |
| and ini_config.has_section("rules") | |
| ): | |
| engine_name = ini_config.get("rules", "engine", fallback="hy") |
| def create_engine( | ||
| config: Any, | ||
| policy: RuleEnginePolicy | None = None, | ||
| store: Any = None, | ||
| ) -> AnalysisRuleEngine: | ||
| """ |
There was a problem hiding this comment.
The factory is intended to support multiple engine implementations via [rules].engine, but the return type is the concrete AnalysisRuleEngine. To keep callers decoupled from the implementation and align with RuleEngineProtocol, consider changing the return annotation to the protocol (or a narrow union) so adding another engine won’t force downstream type changes.
Completes the analysis rule engine (WI-16 and WI-17):
https://claude.ai/code/session_01H5UbjsuiiGya5n1eUCxoaR