Skip to content

feat: add canonical schema proof artifacts and emitted-shape fixtures#38

Merged
khaliqgant merged 5 commits into
mainfrom
miya/relayfile-canonical-schema-proof-pr-v2
Apr 16, 2026
Merged

feat: add canonical schema proof artifacts and emitted-shape fixtures#38
khaliqgant merged 5 commits into
mainfrom
miya/relayfile-canonical-schema-proof-pr-v2

Conversation

@miyaontherelay
Copy link
Copy Markdown
Contributor

@miyaontherelay miyaontherelay commented Apr 15, 2026

Summary

  • add the relayfile canonical schema proof artifacts and validation utility
  • add emitted-shape conformance fixtures plus provenance documentation
  • keep the proof wording honest: adapter path is emitted/authoritative, CLI path is derived supporting evidence

Notes

  • this PR preserves changes that were mistakenly pushed directly to main and then reverted from main
  • review this PR as the proper branch-based path for that work

Validation

  • go test ./internal/schema/...
  • go build ./...
  • workflow 058 completed successfully

Open with Devin

miyaontherelay and others added 4 commits April 16, 2026 01:36
…tes, schema descriptions

- Add ErrUnknownPath sentinel error to ValidateContent so callers can
  distinguish "not validated" from "valid" via errors.Is
- Add description fields to author and state properties in issue.schema.json
- Add path migration note in schemas/README.md for old {number}.json path
- Fix stale doc states: remediation boundary/checklist marked Complete,
  emitted-shape boundary marked Implemented, all checklist boxes checked
- Add merge-approval note to emitted-shape review verdict
- Add TODO comments for compiler architecture improvements in validate.go

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…criptions, doc states

- Add ErrUnknownPath sentinel error to ValidateContent() so callers can
  distinguish "not validated" from "valid" via errors.Is
- Add description fields to author, state, labels, assignees properties
  in issue.schema.json documenting casing conventions and flattening
- Add migration note in schemas/README.md for old {number}.json path
- Fix stale doc states: update "Proposed"/"Defined" to "Implemented"/"Complete"
- Check off completed checklist items in remediation and conformance docs
- Clarify emitted-shape review verdict as "approved for merge"
- Add TODO comments for compiler architecture nits (global compilerErr,
  dual compiler path) to address when second schema is added

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 new potential issues.

View 7 additional findings in Devin Review.

Open in Devin Review

- Embeds JSON Schema files via `schemas/embed.go` (`//go:embed README.md github/*.json`).
- Uses `santhosh-tekuri/jsonschema/v6` with draft 2020-12 and format assertion enabled.
- Path pattern matching via regex: `^/github/repos/[^/]+/[^/]+/issues/\d+/meta\.json$`.
- Returns `nil` for unknown paths — unregistered paths pass silently.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Spec document claims ValidateContent() returns nil for unknown paths, but code returns ErrUnknownPath

The proof-direction doc states ValidateContent() "Returns nil for unknown paths — unregistered paths pass silently." However, the actual implementation at internal/schema/validate.go:46-47 returns fmt.Errorf("%w: %s", ErrUnknownPath, path), and the test TestValidateContentUnknownPath at internal/schema/validate_test.go:128-135 explicitly asserts errors.Is(err, ErrUnknownPath). The authoritative boundary doc (docs/canonical-file-schema-ownership-boundary.md:119) was corrected to say ErrUnknownPath, but this file was missed. This same stale claim also appears in docs/first-canonical-schema-proof-boundary.md:92, docs/first-canonical-schema-proof-plan.md:132, and docs/first-canonical-schema-proof-checklist.md:67. Consumers coding against these specs would fail to handle ErrUnknownPath errors.

Suggested change
- Returns `nil` for unknown paths — unregistered paths pass silently.
- Returns `ErrUnknownPath` for unknown paths (no schema registered) — callers can distinguish "not validated" from "valid" with `errors.Is(err, ErrUnknownPath)`.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

2. The validation utility is in `internal/schema/`, not in `internal/relayfile/`. It does not import `internal/relayfile/` types.
3. Schema files are embedded via `schemas/embed.go` (`//go:embed README.md github/*.json`). No filesystem reads at runtime.
4. Path pattern matching uses regex (`^/github/repos/[^/]+/[^/]+/issues/\d+/meta\.json$`). One pattern registered for this proof.
5. `ValidateContent()` returns `nil` for paths with no registered schema. It does not reject unknown paths.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Boundary Rule #5 states ValidateContent returns nil for unknown paths, contradicting implementation

Boundary Rule #5 states "ValidateContent() returns nil for paths with no registered schema. It does not reject unknown paths." This is a stated design constraint that the implementation is supposed to follow. The actual code at internal/schema/validate.go:46-47 returns ErrUnknownPath, and TestValidateContentUnknownPath at internal/schema/validate_test.go:128-135 asserts that ErrUnknownPath is returned. The rule is factually wrong — the implementation intentionally rejects unknown paths with a sentinel error.

Suggested change
5. `ValidateContent()` returns `nil` for paths with no registered schema. It does not reject unknown paths.
5. `ValidateContent()` returns `ErrUnknownPath` for paths with no registered schema. Callers can distinguish "not validated" from "valid" via `errors.Is(err, ErrUnknownPath)`.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- **Caching**: `sync.Once` for initial compilation of all registered schemas; `sync.Map` for compiled schema cache.
- **Compiler config**: `jsonschema.Draft2020` default draft, `AssertFormat()` enabled (validates `date-time`, `uri`).
- **Error format**: `"validate <path> against <schema>: <detail>"` — includes VFS path, schema file, and JSON Schema validation detail.
- **Unknown paths**: `registeredSchema()` returns `""` -> `ValidateContent()` returns `nil`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Proof plan is internally inconsistent: code snippet says ErrUnknownPath but prose says nil

Within the same file, the code snippet at line 120-121 correctly states "Unknown paths return ErrUnknownPath (checkable via errors.Is)", but the architecture prose at line 132 says "Unknown paths: registeredSchema() returns "" -> ValidateContent() returns nil." The latest commit (f2358b0) fixed the code snippet but missed the prose three paragraphs below it, leaving the document internally contradictory.

Suggested change
- **Unknown paths**: `registeredSchema()` returns `""` -> `ValidateContent()` returns `nil`.
- **Unknown paths**: `registeredSchema()` returns `""` -> `ValidateContent()` returns `ErrUnknownPath`.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- [x] Uses `santhosh-tekuri/jsonschema/v6` with `Draft2020` and `AssertFormat()`
- [x] Exports `ValidateContent(path string, content []byte) error`
- [x] Path pattern matching via regex: `^/github/repos/[^/]+/[^/]+/issues/\d+/meta\.json$`
- [x] Returns `nil` for unregistered path patterns
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Checklist marks 'returns nil for unregistered paths' as complete but implementation returns ErrUnknownPath

The checklist item is checked off ([x]) asserting the implementation "Returns nil for unregistered path patterns." This is factually false — the code at internal/schema/validate.go:46-47 returns ErrUnknownPath and the test at internal/schema/validate_test.go:128-135 asserts errors.Is(err, ErrUnknownPath). A checked-off item that contradicts the actual code undermines the checklist's role as a verification artifact.

Suggested change
- [x] Returns `nil` for unregistered path patterns
- [x] Returns `ErrUnknownPath` for unregistered path patterns (checkable via `errors.Is`)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@khaliqgant khaliqgant merged commit 71e90da into main Apr 16, 2026
6 checks passed
@khaliqgant khaliqgant deleted the miya/relayfile-canonical-schema-proof-pr-v2 branch April 16, 2026 13:12
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