feat: PipeSignature validation#899
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Added PipeSignature and related classes to support contract-only pipe definitions. - Updated PipeSpecUnion to include PipeSignature. - Enhanced WorkingMemoryFactory to create mock Stuff for PipeSignatures. - Modified PipeBlueprint and PipeAbstract to recognize PipeSignature category. - Implemented PipeSignatureBlueprint and PipeSignatureRuntime for runtime behavior. - Added tests for PipeSignature integration and validation. - Updated schema generation to include PipeSignature in definitions.
- Add SignaturesNotAllowedError to handle cases where PipeSignature placeholders are reachable in strict mode. - Update validate_bundle to accept an allow_signatures flag, allowing lenient validation. - Modify dry_run_pipes to respect the allow_signatures flag during execution. - Enhance error messages to provide detailed information about unreachable signatures and their dependency paths. - Introduce tests for validating bundles with signatures in both strict and lenient modes, ensuring correct error handling and success paths. - Update existing tests to accommodate changes in signature validation logic.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f0c25e161
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
3 issues found across 54 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pipelex/pipe_signature/pipe_signature_blueprint.py">
<violation number="1" location="pipelex/pipe_signature/pipe_signature_blueprint.py:19">
P2: `signature_for` is missing blueprint-level validation to reject `PipeType.PIPE_SIGNATURE`, so invalid bundles can bypass the spec guard.</violation>
</file>
<file name="pipelex/cli/agent_cli/commands/validate/_validate_core.py">
<violation number="1" location="pipelex/cli/agent_cli/commands/validate/_validate_core.py:47">
P1: Strict mode silently skips signature pipes instead of rejecting them, which can incorrectly report successful validation when placeholders are present.</violation>
</file>
<file name="pipelex/pipe_run/dry_run.py">
<violation number="1" location="pipelex/pipe_run/dry_run.py:148">
P2: The aggregated strict-mode error can report the wrong `pipe_ref` by always using `pipes[0]`, which may misidentify the offending pipe in the message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR adds contract-only signature pipes and wires them into validation. The main changes are:
Confidence Score: 4/5This is close, but the scoped agent validation path should be fixed before merging.
Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pipelex/cli/agent_cli/commands/validate/_validate_core.py:161-172
**Scoped validation dry-runs everything**
`validate_pipe_in_bundle_core` is meant to validate one selected pipe from a bundle, but it first calls `validate_bundle` with the same strict signature setting. This PR changed `validate_bundle` to dry-run every loaded pipe, so `pipelex-agent validate bundle draft.mthds --pipe real_pipe` can fail before reaching `real_pipe` when the bundle contains an unrelated `PipeSignature`. A user asking to validate an implemented pipe in a mixed draft bundle receives a strict-mode signature failure for pipes outside the requested scope.
Reviews (5): Last reviewed commit: "docs: Sync archived signature TDD plan w..." | Re-trigger Greptile |
Resolves four confirmed issues from cubic, greptile, and codex review on PR #899: - Makefile: make plxt-lint and merge-check-plxt-lint generate the MTHDS schema first, so CI no longer fails on clean checkouts where derived/ is unpopulated. - PipeSignatureBlueprint: reject signature_for=PipeSignature at the language layer (was only guarded at the spec layer). - SignaturesNotAllowedError: carry the set of offending pipe_refs instead of a single pipes[0].pipe_ref, so the aggregated message in dry_run_pipes names the actual offender(s) rather than the first iterated pipe. Multi-offender case uses a plural header. - _validate_pipe_or_bundle (single-pipe path): wrap SignaturesNotAllowedError in a Rich-formatted handler + typer.Exit(1), matching the bundle path's treatment; users no longer see a raw traceback. Includes regression tests for each fix (TDD red-then-green) under tests/unit/pipelex/pipe_signature/, tests/integration/pipelex/pipe_signature/, and tests/integration/pipelex/cli/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sNotAllowedError The property had zero callers — added defensively when the constructor signature flipped to `offending_pipe_refs`. Per project rule "no backward compatibility", remove it now rather than carry dead code. `offending_pipe_refs` is the only remaining identifier for offenders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oving pipe_dependencies and enhancing error handling for signatures
- Renamed `PipeSignature` to `PipeSignatureSpec` to clarify its role as a contract-only pipe. - Introduced a new `PipeSignature` class for runtime execution, which raises errors for unimplemented signatures. - Updated various modules to reflect the new naming and structural changes, including `pipe_spec_map.py`, `pipe_spec_union.py`, and `registry_models.py`. - Enhanced the `PipeSignatureSpec` with detailed documentation and validation logic. - Modified tests to accommodate the new structure and ensure proper functionality of the signature system. - Updated related documentation to reflect changes in the signature handling and validation processes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ands to support PipeSignature placeholders
# Conflicts: # CHANGELOG.md # TODOS.md
…alk module The collect_signature_refs / collect_signature_paths walk previously lived on PipeAbstract and took a pipe_lookup callable parameter to dodge the pipe_abstract -> hub -> library -> pipe_library -> pipe_abstract import cycle. That parameter was a code smell. Move the walk into a new module of free functions, pipelex/pipe_signature/signature_walk.py, which sits downstream of pipelex.hub and so imports get_optional_pipe directly with no cycle. PipeAbstract keeps only pipe_dependencies() and is_signature. Recursion, visited-set cycle protection, sorted iteration, and the longest/first-path semantics are preserved exactly. All call sites and tests updated; CHANGELOG corrected. Also archive the signature-based-validation TDD plan to wip/archive/ and add a PR-story HTML page briefing the work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a dated post-archive update note recording that the graph walk moved off PipeAbstract into pipelex/pipe_signature/signature_walk.py, and correct the now-stale collect_signature_refs(pipe_lookup=...) references in the always-current sections (Current state, architecture sketch, file reference). The per-phase deviation notes are left verbatim as the historical implementation record. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| """ | ||
| # Validate the bundle to load all its pipes into the library | ||
| # This ensures all dependencies are available | ||
| await validate_bundle(mthds_file_path=bundle_path, library_dirs=library_dirs) | ||
| await validate_bundle( | ||
| mthds_file_path=bundle_path, | ||
| library_dirs=library_dirs, | ||
| allow_signatures=allow_signatures, | ||
| ) | ||
|
|
||
| # Now get the specific pipe and dry-run only that one | ||
| the_pipe = get_required_pipe(pipe_code=pipe_code) | ||
| await dry_run_pipe(the_pipe, raise_on_failure=True) | ||
| await dry_run_pipe(the_pipe, allow_signatures=allow_signatures, raise_on_failure=True) |
There was a problem hiding this comment.
Scoped validation dry-runs everything
validate_pipe_in_bundle_core is meant to validate one selected pipe from a bundle, but it first calls validate_bundle with the same strict signature setting. This PR changed validate_bundle to dry-run every loaded pipe, so pipelex-agent validate bundle draft.mthds --pipe real_pipe can fail before reaching real_pipe when the bundle contains an unrelated PipeSignature. A user asking to validate an implemented pipe in a mixed draft bundle receives a strict-mode signature failure for pipes outside the requested scope.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pipelex/cli/agent_cli/commands/validate/_validate_core.py
Line: 161-172
Comment:
**Scoped validation dry-runs everything**
`validate_pipe_in_bundle_core` is meant to validate one selected pipe from a bundle, but it first calls `validate_bundle` with the same strict signature setting. This PR changed `validate_bundle` to dry-run every loaded pipe, so `pipelex-agent validate bundle draft.mthds --pipe real_pipe` can fail before reaching `real_pipe` when the bundle contains an unrelated `PipeSignature`. A user asking to validate an implemented pipe in a mixed draft bundle receives a strict-mode signature failure for pipes outside the requested scope.
How can I resolve this? If you propose a fix, please make it concise.
Summary
PipeSignature, a contract-only pipe type that declares inputs/outputs without an implementation — letting bundles validate against signatures of pipes that live elsewhere.PipeSignatureend-to-end: builder spec, blueprint union, runtime hydration, dry-run handling, CLI commands (validate bundle,validate pipe), and the agent CLI..pipelex/plxt.tomlpoints atderived/mthds_schema.jsonso.mthdsfiles on this branch validate against the schema that knows aboutPipeSignature. Removed at merge time oncevscode-pipelexships the updated bundled schema (tracked in TODOS Phase 7.4).docs/building-methods/pipes/signature-pipes.md, updatedvalidateCLI page, mkdocs nav entry.Test plan
make agent-checkmake agent-test🤖 Generated with Claude Code
Summary by cubic
Adds
PipeSignature, a contract-only pipe for designing pipelines before implementation, with strict-by-default validation, lenient mode via--allow-signatures, dry-run mocks, and clearer CLI reporting. Also moves signature detection intopipelex/pipe_signature/signature_walk.pyto avoid import cycles and keepPipeAbstractminimal.New Features
PipeSignatureSpecadded toPipeSpecUnion; runtimePipeSignature+PipeSignatureFactory/PipeSignatureBlueprintregistered end-to-end (schema, unions, registries).--allow-signaturesonpipelexandpipelex-agentfor pipe/method/bundle. Lenient summaries append a signature count.PipeSignatureNotExecutableError.validate pipe|method|bundleaccept--allow-signaturesin both CLIs, with improved output. Branch-local.pipelex/plxt.tomlpoints toderived/mthds_schema.json.signature_walk.pyrefactor.Bug Fixes
signature_for=PipeSignatureat both spec and blueprint layers.SignaturesNotAllowedErroraggregates all offending pipe refs; both single-pipe and bundle paths show a Rich-formatted “Unimplemented Signatures” section and exit cleanly.plxt-lintandmerge-check-plxt-lintgenerate the MTHDS schema first to fix CI on clean checkouts.PipeAbstractintopipelex/pipe_signature/signature_walk.py; update all call sites and tests.Written for commit a10ba98. Summary will update on new commits. Review in cubic
Documentation
Audited every docs file against the diff. Feature documentation was already thorough — the branch shipped a new page, nav entry, cross-links, and CLI reference.
/document-releasefound one gap and fixed it:docs/tools/cli/agent-cli.md: added a "Signature pipes" note to the Validate section —pipelex-agent validateruns lenient validation by default (acceptsPipeSignatureplaceholders, no flag to make it strict), unlikepipelex validatewhich is strict by default. This behavioral difference was undocumented.Already in place on the branch, verified accurate:
docs/building-methods/pipes/signature-pipes.md— new page: when to use, runtime behavior table, MTHDS parameters, examples (single contract, signature in a sequence, multiplicity), CLI usage, replacement workflow.docs/building-methods/pipes/index.md— paragraph introducing signature pipes as the third kind of pipe.docs/tools/cli/validate.md—--allow-signaturesflag documented for bothvalidate pipeandvalidate bundle, with examples.mkdocs.yml— nav + plugin entry for the new page.pipelex/builder/CLAUDE.md—pipe_signature.py→pipe_signature_spec.pyrename reflected.CHANGELOG.md—[Unreleased]Added/Changed entries coverPipeSignature,--allow-signatures, the graph walk, and the threadedallow_signaturesparameter.Documentation coverage
Common gap (not blocking): no step-by-step tutorial walkthrough for newcomers building their first signature-stubbed pipeline. The
signature-pipes.mdexamples cover the how-to quadrant well; a guided tutorial could be added later via/document-generateif signature-first authoring becomes a promoted workflow.No architecture diagrams reference signature internals — no diagram drift.