Fix #13: tailoring, atomic-skill, and setup gaps#16
Merged
Conversation
…nk names The previous Step 2b inferred chain direction from a heuristic table mapping link option names to fixed chains (e.g., `implements -> "spec -> impl"`). The table assumed a single convention (parent references child). On projects that follow the opposite convention (child references parent via `:implements:`), every emitted chain is the inverse of the actual edge and `pharaoh:mece` reports 100% gaps for the source type. Replace the heuristic table with three sources applied in priority order: 1. Built `needs.json` when available -- emit `X -> Y` only where at least 90% of `X` instances with the link option resolve to a `Y` (and at least 3 instances exist), so the direction matches the data the project emits. 2. Declared `outgoing`/`incoming` semantics in `[needs.links.<name>]` -- only used when an explicit type-pair hint is supplied; the description alone does not identify the type pair. 3. Refuse to guess -- emit a TODO comment instead of a chain. The type-pair declared-in-`[[needs.types]]` filter and the empty-array fallback continue to apply to every source. Closes #11
…guidance (#12) The 71 .github/agents/pharaoh.*.agent.md files shipped with descriptions that diverged from their source SKILL.md. Two were catastrophically truncated at the first '.' (split caught 'e.g.' and 'index.rst'), producing unclosed parens and backticks that broke Markdown rendering in Copilot's review UI on downstream PRs (sphinx-needs-demo#51). Changes: - 61 thin-wrapper agents: regenerate description and body verbatim from SKILL.md frontmatter; preserve existing handoffs. - 8 fat agents (mece, plan, change, spec, setup, decide, release, trace): update only the frontmatter description; leave hand-tailored inline content untouched. - pharaoh.setup.agent.md Step 3: rewrite gitignore guidance to mirror SKILL.md Step 4b. Ignore only ephemeral subpaths (.pharaoh/runs/, .pharaoh/plans/, .pharaoh/session.json, .pharaoh/cache/); .pharaoh/project/ tailoring stays tracked. - ci.yaml: add a guard that fails the agent-frontmatter step when the description has unbalanced parens, brackets, or backticks. Catches the truncation class that produced the original regression.
Removes six references in pharaoh-arch-review/SKILL.md and one in pharaoh-arch-draft/SKILL.md to a planned-but-never-implemented pharaoh-arch-regenerate skill: chains_from list, prose handoff hints, and three deferred rubric axes. Re-authoring after review is a follow-up step outside the review skill's scope. If we later want regenerate symmetry between req and arch (i.e. ship pharaoh-arch-regenerate as a sibling to pharaoh-req-regenerate), that is a separate PR. Closes part of #13 (section 6).
"Confirm the findings clear" parses as a sentence missing a verb; "confirm the findings are resolved" reads cleanly. Caught in PR review.
bburda
changed the title
There was a problem hiding this comment.
Pull request overview
Primarily a documentation/metadata cleanup across Pharaoh skill and agent definitions, with one CI validation addition and a broader rewrite of pharaoh-setup guidance. In the codebase, these changes affect how agent cards are described, how setup is documented, and what CI now validates for agent frontmatter.
Changes:
- Removes
pharaoh-arch-regeneratereferences from architecture draft/review docs. - Refreshes many
.github/agents/*.agent.mddescriptions and fixes previously truncated agent metadata. - Adds CI checks for unbalanced delimiters in agent
descriptionfields and expandspharaoh-setupguidance around traceability inference.
Reviewed changes
Copilot reviewed 75 out of 75 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
skills/pharaoh-setup/SKILL.md |
Rewrites required_links inference guidance for generated pharaoh.toml. |
skills/pharaoh-arch-review/SKILL.md |
Removes dangling arch-regenerate references and updates review follow-up wording. |
skills/pharaoh-arch-draft/SKILL.md |
Replaces regenerate guidance in the final self-review step. |
.github/workflows/ci.yaml |
Adds delimiter-balance validation for agent frontmatter descriptions. |
.github/agents/pharaoh.write-plan.agent.md |
Fixes previously truncated agent description text. |
.github/agents/pharaoh.vplan-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.vplan-draft.agent.md |
Expands agent description text. |
.github/agents/pharaoh.use-case-diagram-draft.agent.md |
Expands agent description text. |
.github/agents/pharaoh.trace.agent.md |
Refreshes top-level trace agent description. |
.github/agents/pharaoh.toctree-emit.agent.md |
Fixes previously truncated agent description text. |
.github/agents/pharaoh.tailor-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.tailor-fill.agent.md |
Expands agent description text. |
.github/agents/pharaoh.tailor-detect.agent.md |
Expands agent description text. |
.github/agents/pharaoh.tailor-code-grounding-filters.agent.md |
Expands agent description text. |
.github/agents/pharaoh.tailor-bootstrap.agent.md |
Expands agent description text. |
.github/agents/pharaoh.status-lifecycle-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.state-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.standard-conformance.agent.md |
Expands agent description text. |
.github/agents/pharaoh.sphinx-extension-add.agent.md |
Expands agent description text. |
.github/agents/pharaoh.spec.agent.md |
Refreshes top-level spec agent description. |
.github/agents/pharaoh.setup.agent.md |
Refreshes top-level setup description and .gitignore guidance. |
.github/agents/pharaoh.sequence-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.self-review-coverage-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.review-completeness.agent.md |
Expands agent description text. |
.github/agents/pharaoh.req-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.req-regenerate.agent.md |
Expands agent description text. |
.github/agents/pharaoh.req-from-code.agent.md |
Expands agent description text. |
.github/agents/pharaoh.req-draft.agent.md |
Expands agent description text. |
.github/agents/pharaoh.req-codelink-annotate.agent.md |
Expands agent description text. |
.github/agents/pharaoh.req-code-grounding-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.reproducibility-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.release.agent.md |
Refreshes top-level release agent description. |
.github/agents/pharaoh.quality-gate.agent.md |
Expands agent description text. |
.github/agents/pharaoh.prose-migrate.agent.md |
Expands agent description text. |
.github/agents/pharaoh.process-audit.agent.md |
Expands agent description text. |
.github/agents/pharaoh.plan.agent.md |
Refreshes top-level plan agent description. |
.github/agents/pharaoh.papyrus-non-empty-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.output-validate.agent.md |
Expands agent description text. |
.github/agents/pharaoh.mece.agent.md |
Refreshes top-level MECE agent description. |
.github/agents/pharaoh.link-completeness-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.lifecycle-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.id-convention-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.id-allocate.agent.md |
Expands agent description text. |
.github/agents/pharaoh.gate-advisor.agent.md |
Expands agent description text. |
.github/agents/pharaoh.fmea.agent.md |
Expands agent description text. |
.github/agents/pharaoh.fmea-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.flow.agent.md |
Expands agent description text. |
.github/agents/pharaoh.finding-record.agent.md |
Expands agent description text. |
.github/agents/pharaoh.feat-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.feat-flow-extract.agent.md |
Expands agent description text. |
.github/agents/pharaoh.feat-file-map.agent.md |
Expands agent description text. |
.github/agents/pharaoh.feat-draft-from-docs.agent.md |
Expands agent description text. |
.github/agents/pharaoh.feat-component-extract.agent.md |
Expands agent description text. |
.github/agents/pharaoh.feat-balance.agent.md |
Expands agent description text. |
.github/agents/pharaoh.fault-tree-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.execute-plan.agent.md |
Expands agent description text. |
.github/agents/pharaoh.dispatch-signal-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.diagram-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.diagram-lint.agent.md |
Expands agent description text. |
.github/agents/pharaoh.deployment-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.decision-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.decision-record.agent.md |
Expands agent description text. |
.github/agents/pharaoh.decide.agent.md |
Refreshes top-level decide agent description. |
.github/agents/pharaoh.coverage-gap.agent.md |
Expands agent description text. |
.github/agents/pharaoh.context-gather.agent.md |
Expands agent description text. |
.github/agents/pharaoh.component-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.class-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.change.agent.md |
Refreshes top-level change agent description. |
.github/agents/pharaoh.bootstrap.agent.md |
Expands bootstrap agent description text. |
.github/agents/pharaoh.block-diagram-draft.agent.md |
Expands planned-skill agent description. |
.github/agents/pharaoh.audit-fanout.agent.md |
Expands agent description text. |
.github/agents/pharaoh.arch-review.agent.md |
Expands agent description text. |
.github/agents/pharaoh.arch-draft.agent.md |
Expands agent description text. |
.github/agents/pharaoh.api-coverage-check.agent.md |
Expands agent description text. |
.github/agents/pharaoh.activity-diagram-draft.agent.md |
Expands planned-skill agent description. |
| ## Last step | ||
|
|
||
| After emitting the artefact, invoke `pharaoh-arch-review` on it. Pass the emitted artefact (or its `need_id`) as `target`. Attach the returned review JSON to the skill's output under the key `review`. If the review emits any axis with `score: 0` or `severity: critical`, return a non-success status with the review findings verbatim and do NOT finalize the artefact — the caller must regenerate (via `pharaoh-arch-regenerate` if available, or by re-invoking this skill with the findings as input). | ||
| After emitting the artefact, invoke `pharaoh-arch-review` on it. Pass the emitted artefact (or its `need_id`) as `target`. Attach the returned review JSON to the skill's output under the key `review`. If the review emits any axis with `score: 0` or `severity: critical`, return a non-success status with the review findings verbatim and do NOT finalize the artefact — the caller must address the action items and re-invoke this skill with the revised target as input. |
| .pharaoh/cache/ | ||
| ``` | ||
|
|
||
| If `.gitignore` already contains a bare `.pharaoh/` (or `.pharaoh`) line, leave it alone and warn the user that the wide form hides `.pharaoh/project/` tailoring which should be committed; recommend narrowing to the four ephemeral entries above. Do not auto-migrate — respect user control. |
Comment on lines
+91
to
+106
| description=$(echo "$frontmatter" | sed -n 's/^description:[[:space:]]*//p') | ||
| opens=$(echo -n "$description" | tr -cd '(' | wc -c) | ||
| closes=$(echo -n "$description" | tr -cd ')' | wc -c) | ||
| if [ "$opens" -ne "$closes" ]; then | ||
| echo "ERROR: $agent_file 'description' has unbalanced parentheses ($opens '(' vs $closes ')')" | ||
| status=1 | ||
| fi | ||
| obrack=$(echo -n "$description" | tr -cd '[' | wc -c) | ||
| cbrack=$(echo -n "$description" | tr -cd ']' | wc -c) | ||
| if [ "$obrack" -ne "$cbrack" ]; then | ||
| echo "ERROR: $agent_file 'description' has unbalanced brackets ($obrack '[' vs $cbrack ']')" | ||
| status=1 | ||
| fi | ||
| ticks=$(echo -n "$description" | tr -cd '`' | wc -c) | ||
| if [ $((ticks % 2)) -ne 0 ]; then | ||
| echo "ERROR: $agent_file 'description' has unbalanced backticks ($ticks total)" |
Comment on lines
+252
to
+288
| `required_links` declares chains in the form `"source-type -> target-type"`. The semantics enforced by `pharaoh:mece` are: every need of `source-type` must have at least one outgoing link to a need of `target-type` (see `skills/pharaoh-mece/SKILL.md` Step 2). The chain direction is therefore the direction the link option resolves, **not** the direction of the conceptual type hierarchy. Both conventions exist in the wild — some projects put `:implements:` on the `impl` directive (child references parent, chain `impl -> spec`); others put `:specifies:` on the `spec` directive (parent references child, chain `spec -> impl`). Inferring direction from the link option name picks one convention and emits inverted chains for projects on the other; `pharaoh:mece` then reports 100% gaps for the source type. Resolve direction from ground truth instead. | ||
|
|
||
| Apply the following sources in priority order. For each link option, stop at the first source that resolves a direction. | ||
|
|
||
| **Source 1 — built `needs.json` (preferred, when available).** If the project has a built `needs.json` (typical paths: `<source-dir>/_build/needs/needs.json`, `<source-dir>/_build/html/needs.json`, or any `needs.json` under `_build/`), parse it and inspect the actual edges: | ||
|
|
||
| For each declared link option `L` (including the standard `:links:`) and each ordered pair of declared types `(X, Y)`: | ||
|
|
||
| 1. Let `n_X` = count of needs of type `X` whose `:L:` value is non-empty. | ||
| 2. Let `n_X_to_Y` = count of those needs whose `:L:` resolves to at least one need of type `Y`. | ||
| 3. Emit `"X -> Y"` only if `n_X >= 3` and `n_X_to_Y / n_X >= 0.9`. | ||
|
|
||
| The thresholds (`>= 3` instances, `>= 90%` coverage) suppress chains inferred from a single accidental edge while still emitting chains the project consistently produces. Include a comment recording the sample size: | ||
|
|
||
| ```toml | ||
| required_links = [ | ||
| "spec -> req", # needs.json: 18/18 spec needs link to req via :reqs: | ||
| "impl -> spec", # needs.json: 35/35 impl needs link to spec via :implements: | ||
| ] | ||
| ``` | ||
|
|
||
| **Source 2 — declared semantics from `[needs.links.<name>]` (greenfield, no `needs.json`).** The `outgoing` and `incoming` descriptions identify the verb and which side bears the link option, but they do not, on their own, identify the type pair: any type can carry any link option. Without empirical edges or an explicit hint, the source and target types are unknown. | ||
|
|
||
| Use this source only when `ubproject.toml` carries an explicit hint (e.g., a future `[needs.links.<name>] from = "<type>", to = "<type>"` extension). Do not invent the type pair from the link option name. | ||
|
|
||
| **Source 3 — refuse to guess.** When neither source resolves a link option to a `(source-type, target-type)` pair, do **not** emit a chain for that link. Emit a TODO comment in its place so the user sees what was skipped and why: | ||
|
|
||
| ```toml | ||
| required_links = [ | ||
| "spec -> req", # needs.json: 18/18 spec needs link to req via :reqs: | ||
| # TODO: link option `implements` is declared but no `needs.json` was | ||
| # found. Build the docs once and re-run `pharaoh:setup`, or add an | ||
| # explicit chain manually in the form "source-type -> target-type". | ||
| ] | ||
| ``` | ||
|
|
||
| The previous heuristic-name table (`implements -> "spec -> impl"`, `tests -> "impl -> test"`, etc.) is removed: it encoded one project convention as universal and produced inverted chains on every project that used the opposite convention. |
Comment on lines
+300
to
+307
| **Empty-array fallback.** If no link option resolves to a chain by any source, emit: | ||
|
|
||
| ```toml | ||
| required_links = [ | ||
| # No traceability chains inferred. Add entries of the form | ||
| # "source-type -> target-type" once the link conventions stabilise, | ||
| # or build the docs and re-run `pharaoh:setup` to infer from `needs.json`. | ||
| ] |
| @@ -1,10 +1,10 @@ | |||
| --- | |||
| description: Use when drafting one component-relationship diagram (nodes = sphinx-needs, edges = link relations) for a bounded scope — one feature, one module, one architectural view. | |||
| description: Use when drafting one component-relationship diagram (nodes = sphinx-needs, edges = link relations) for a bounded scope — one feature, one module, one architectural view. Renderer tailored via `pharaoh.toml`. Does NOT emit sequence, class, or state diagrams — those are separate skills. Status — PLANNED (design-only scaffold; invoking returns sentinel FAIL until implemented). | |||
| @@ -1,10 +1,10 @@ | |||
| --- | |||
| description: Use when drafting one deployment diagram showing physical nodes (ECUs, servers, boards), the software artefacts deployed on each, and communication channels (buses, networks). | |||
| description: Use when drafting one deployment diagram showing physical nodes (ECUs, servers, boards), the software artefacts deployed on each, and communication channels (buses, networks). Typical ASPICE usage — SYS.3 System Architectural Design; essential for automotive HW/SW allocation per ISO 26262 Part 5 (HW) and Part 6 (SW). Renderer tailored via `pharaoh.toml`. Status — PLANNED (design-only scaffold; invoking returns sentinel FAIL until implemented). | |||
| @@ -1,10 +1,10 @@ | |||
| --- | |||
| description: Use when drafting one fault tree for FTA (Fault Tree Analysis) — a top hazard event decomposed through AND/OR gates into basic events (component failures, random hardware faults, human errors). | |||
| description: Use when drafting one fault tree for FTA (Fault Tree Analysis) — a top hazard event decomposed through AND/OR gates into basic events (component failures, random hardware faults, human errors). Typical ISO 26262 usage — Part 3 Hazard Analysis & Risk Assessment, and Part 5 supporting hardware architectural metrics. Renderer tailored via `pharaoh.toml`. Status — PLANNED (design-only scaffold; invoking returns sentinel FAIL until implemented). | |||
| @@ -1,10 +1,10 @@ | |||
| --- | |||
| description: Use when drafting one sequence diagram showing ordered interactions between participants (components, actors, external systems) over time. | |||
| description: Use when drafting one sequence diagram showing ordered interactions between participants (components, actors, external systems) over time. Renderer tailored via `pharaoh.toml`. Does NOT emit component, class, or state diagrams. Status — PLANNED (design-only scaffold; invoking returns sentinel FAIL until implemented). | |||
| @@ -1,10 +1,10 @@ | |||
| --- | |||
| description: Use when drafting one state-machine diagram showing lifecycle or behavioral states of a component/entity, with labeled transitions. | |||
| description: Use when drafting one state-machine diagram showing lifecycle or behavioral states of a component/entity, with labeled transitions. Renderer tailored via `pharaoh.toml`. Does NOT emit component, sequence, or class diagrams. Status — PLANNED (design-only scaffold; invoking returns sentinel FAIL until implemented). | |||
Implements the two user-entry slash commands @pharaoh.author and @pharaoh.verify that copilot-instructions.md documents as canonical steps in the workflow chain (@pharaoh.plan -> @pharaoh.change -> @pharaoh.author -> @pharaoh.verify -> @pharaoh.release). Both were dead before: prompt files only carried agent-frontmatter pointing at agent files that did not exist, while ~26 live references in 6 sibling agent.md files plus copilot-instructions.md called into them. pharaoh-author is a type-routing orchestrator over the existing atomic drafting skills (pharaoh-req-draft, pharaoh-arch-draft, pharaoh-vplan- draft, pharaoh-decide). Dispatch entries for pharaoh-arch-draft and pharaoh-vplan-draft are deliberately thin passthroughs -- a follow-up in this PR will update them once those skills' interfaces are parameterized. pharaoh-verify is a content-level satisfaction checker -- given a need-id, walks the :satisfies: chain and evaluates whether the child addresses the parent's substance. Distinct from pharaoh-mece (structural), pharaoh-req-review (per-axis rubric), and pharaoh-tailor-review (schema-level). Closes part of #13 (section 5).
Moves the four JSON Schemas from examples/score/.pharaoh/project/schemas/ to schemas/ at the package root so every Pharaoh-tailored project shares the same authoritative validation rules. Adds schemas/README.md documenting the resolution order and per-file responsibilities. Updates pharaoh-tailor-review to resolve schemas in priority order: (1) explicit schemas_dir argument, (2) per-project <tailoring_dir>/schemas/, (3) the shipped schemas/ folder, (4) fallback to built-in structural rules with degraded_mode flag. Fixes pharaoh-tailor-bootstrap output to validate against the canonical artefact-catalog schema by dropping the unused child_of and lifecycle_ref keys (both rejected by additionalProperties: false). The information child_of carried is recoverable from artefact-type link declarations; lifecycle_ref was decorative. Updates skills/shared/tailoring-access.md to document only the schema-allowed fields (required_fields, optional_fields, lifecycle, required_body_sections) — the prior child_of and lifecycle_ref entries were inconsistent with the canonical schema. Closes part of #13: section 1 sub-bullets (a) artefact-catalog additionalProperties violations and (d) schemas at canonical install path. Sub-bullets (b) and (c) are addressed by the next commit on this branch.
bburda
added a commit
to bburda/sphinx-needs-demo
that referenced
this pull request
May 5, 2026
Pulls the agent and prompt template state from the upstream PR that addresses my consolidated feedback (useblocks/pharaoh#13). Adds the two missing user-entry agents (@pharaoh.author, @pharaoh.verify) so the matching slash commands stop dispatching to nothing, and picks up the description and gitignore-guidance fixes from the same upstream branch (covers useblocks/pharaoh#11 and useblocks#12 in one shot). Replaces the local hand-edits applied earlier in this PR for the five agent files I had patched (pharaoh.write-plan, pharaoh.toctree-emit, pharaoh.execute-plan, pharaoh.feat-file-map, pharaoh.setup) with the upstream versions, so the templates stay in sync with the plugin.
Aligns pharaoh-tailor-bootstrap and pharaoh-tailor-fill so the same
tailoring file gets the same shape regardless of which authoring path
ran. Resolves divergences caught when validating real-world projects:
workflows.yaml: bootstrap rewrites its per-type {from, to, gate} map
output to the schema-shaped flat lifecycle_states + transitions[*]
{from, to, requires}. fill already emits this shape on the transitions
side but printed lifecycle_states as a description map; corrected to a
flat list per the schema. bootstrap was the outlier on transitions.
id-conventions.yaml id_regex_*: pharaoh-id-convention-check now reads
id_regex_exceptions (the schema key) instead of id_regex_by_type. The
three fixture files in skills/pharaoh-id-convention-check/fixtures/
move to the same key.
id-conventions.yaml prefixes: canonical value form is the identifier
prefix string (e.g. REQ_, gd_req), not a human description. Tightens
the prefixes additionalProperties regex in
schemas/id-conventions.schema.json to enforce identifier-prefix shape
and updates pharaoh-tailor-fill plus the score example tailoring to
emit prefix strings instead of prose descriptions.
pharaoh-lifecycle-check: documentation note tying the workflows.yaml
shape it consumes to schemas/workflows.schema.json; no behavioural
change.
Closes #13 sections 2 (bootstrap/fill incompatibility) and 1 sub-bullets
(b) workflows shape mismatch and (c) id-conventions value-form ambiguity.
Closes the silent no-op where four release-gate fields were consumed but never emitted by any tailoring author: required_links / optional_links read by pharaoh-link-completeness-check required_metadata_fields read by pharaoh-output-validate required_roles read by pharaoh-review-completeness Adds all four as optional per-type properties to schemas/artefact-catalog.schema.json. Documents inference rules in pharaoh-tailor-bootstrap and emission defaults in pharaoh-tailor-fill. Adds cross-file rule C6 in pharaoh-tailor-review that surfaces a finding when any of the three required-* fields is absent (empty arrays are valid -- they declare an explicit choice; missing keys are not). Closes #13 (section 3).
…aults source
Adds the three sections that were consumed but undocumented:
[pharaoh.codelink_comments] read by pharaoh-req-codelink-annotate
and pharaoh-req-from-code
[pharaoh.diagrams] read by 11 diagram-draft skills plus
pharaoh-feat-component-extract and
pharaoh-feat-flow-extract
[pharaoh.quality_gate] read by pharaoh-diagram-lint
Documents [pharaoh.codelinks].src_dir (consumed by pharaoh-write-plan
and pharaoh-feat-file-map) which the example previously left
undocumented under [pharaoh.codelinks].
Reconciles the workflow-flag defaults that previously disagreed across
three sources (shared/strictness.md, pharaoh-gate-advisor, the
example). The example is now the single source of truth; the other two
defer to it explicitly rather than redeclare.
Persists [pharaoh.workflow].mode as a real key (was a comment-only line
that no skill could parse).
Closes #13 (section 4).
…rget_level
Drops the hardcoded type vocabulary that locked these two atomic
drafting skills to a fixed set of artefact types:
pharaoh-arch-draft no longer accepts only arch_type in {module,
component, interface}. It now takes a target_level argument and reads
the type's required_fields and id_regex from artefact-catalog.yaml /
id-conventions.yaml. Drafting swarch, sys-arch, or any other
architecture-shaped need declared in the catalog now works without
modifying the skill.
pharaoh-vplan-draft no longer hardcodes the tc__ prefix or the tc
catalog key. The prefix is derived from id-conventions.yaml's prefixes
map for the target_level type; the catalog key matches the type name.
A project whose test type uses prefix T_ now generates compliant IDs.
pharaoh-author routing entries for both skills now pass the
parameterized target_level instead of the previous thin passthrough.
The router dispatches by artefact-catalog category, so any
catalog-declared architecture type routes to pharaoh-arch-draft and
any verification-plan-shaped type routes to pharaoh-vplan-draft.
Closes #13 section 9 sub-bullets (a) and (b).
Reframes pharaoh-req-draft as the canonical drafter for any requirement-shaped artefact, not just classical req / comp_req. Any target_level declared in artefact-catalog.yaml routes here, including ISO 26262 safety-V types: hazard (HARA outputs), safety_goal (Part 3), and fsr / functional safety requirement (Part 4). The skill drives every type from artefact-catalog.yaml's required_fields and required_metadata_fields, so safety-V-specific fields like asil, severity, exposure, controllability, and safe_state are surfaced as placeholders without hardcoding ISO-26262 logic into the skill itself. A project that doesn't declare these fields in its catalog gets a plain requirement; a project that does (e.g. useblocks/sphinx-needs-demo) gets the safety-V shape. pharaoh-author routing for safety-V target_levels no longer FAILs with "no drafter for type X yet" -- they dispatch to the reframed pharaoh-req-draft like any other requirement-shaped type. Closes #13 section 9 sub-bullet (c).
Today the orchestrator's description claims V-model end-to-end but
the body only walks the classical req -> arch -> vplan -> fmea chain.
swreq / sysreq / sys-arch / swarch never appear; hazard / safety_goal
/ fsr never appear.
Extends the chain to three optional layers:
Safety V: hazard -> safety_goal -> fsr (ISO 26262 Part 3 + 4)
SYS/SWE: sysreq -> sys-arch
swreq -> swarch
Classical: req / comp_req -> arch -> vplan -> fmea
Each layer auto-detects from artefact-catalog.yaml: a layer runs only
if the catalog declares its types. Caller can also pass an explicit
stages argument to skip layers (e.g. run only safety_v while the rest
of the V is still being bootstrapped).
Safety-V dispatch goes to pharaoh-req-draft with the appropriate
target_level (hazard / safety_goal / fsr), not new safety-V skills —
the reframe in the prior commit makes pharaoh-req-draft the canonical
drafter for any requirement-shaped artefact.
Closes #13 section 10.
pharaoh-setup now consults the project's declared conventions and
existing artefacts before generating tailoring. Six setup defects
on real-world projects (useblocks/sphinx-needs-demo) drove this:
- artefact-catalog optional_fields seeded from ubproject.toml
[needs.fields.X], not from a Pharaoh-internal placeholder set
- workflows.yaml lifecycle_states derived from a status histogram of
existing RST files; falls back to draft -> reviewed -> approved
only when no project history exists
- id-conventions prefixes detect collisions in [[needs.types]] and
warn or FAIL with a remediation hint
- id_regex shape detected from existing IDs (e.g. {DOMAIN}_{NUMBER}),
not the prescriptive {TYPE}_{NUMBER} default
- mode classifier uses [[needs.types]] declarations and RST content,
not the gitignored needs.json build artefact
- release-gate fields from the canonical schema (required_links,
optional_links, required_metadata_fields, required_roles) are
populated per type from observed link/field declarations
required_links direction inference, partially shipped in PR #14,
extended to cover the full set of relations declared in
[needs.links.X].
Closes #13 section 8.
…e release-gate fields pharaoh-author Step 2 dispatched to pharaoh-req-draft without target_level, so a call like pharaoh-author(target_type=hazard) would fall through pharaoh-req-draft's "default to first req-shaped catalog key" path and emit a gd_req instead of a hazard. Same problem for safety_goal, fsr, tsr, sysreq, swreq, and any non-default req variant. The Step 1 routing table already advertised "target_level forwarded verbatim" — this commit makes the dispatch block honour that. Also populates the four release-gate fields (required_links, optional_links, required_metadata_fields, required_roles) for every type entry in the score example artefact-catalog. The schema added the fields as optional, the bootstrap and setup skills emit them, but the canonical example file was silent — leaving a copy-paste source that disagrees with what fresh tailoring runs would produce.
Critical: - CHANGELOG.md drops local pharaoh-validation harness citations - pharaoh-tailor-review SKILL.md drops harness-cite at line 304 - pharaoh-author drops category-field routing (schema additionalProperties: false rejects it); relies on synonym table only - pharaoh-vplan-draft now defaults verification_level=system on absent input so pharaoh-author dispatch chain doesn't hard-FAIL - pharaoh.setup.agent.md replaces hardcoded 8-agent list with dynamic enumeration from .github/agents/ - pharaoh-tailor-review prefixes-value rule no longer calls it a "description"; matches schema pattern Important: - pharaoh-author resolution rule made deterministic for ambiguous *_req - plugin.json bumped 0.1.0 -> 0.2.0 - CHANGELOG migration section: id_regex_by_type rename, prefixes value form, child_of/lifecycle_ref drops, C6 behaviour - pharaoh.author / pharaoh.verify added to handoffs of pharaoh.change and pharaoh.plan agents
patdhlk
approved these changes
May 5, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 104 out of 106 changed files in this pull request and generated 18 comments.
Comments suppressed due to low confidence (2)
pharaoh.toml.example:35
pharaoh-setup's new mode table says reverse-engineering projects defaultrequire_change_analysistofalse, and the comment above this section says reverse-eng gates start permissive. Leaving the canonical example atmode = "reverse-eng"+require_change_analysis = truereintroduces the competing-defaults problem this PR is trying to remove, because any skill that now treatspharaoh.toml.exampleas the source of truth will infer the opposite behavior.
schemas/id-conventions.schema.json:15- The new value pattern rejects hyphens, which breaks the "use the directive name itself as the prefix" model for the new
sys-archtype names introduced elsewhere in this PR. A project that wants IDs likesys-arch__...(or even just wants to preservesys-archas the prefix token before applyingseparator) can no longer express that inid-conventions.yaml, even thoughpharaoh-arch-draftandpharaoh-flownow advertisesys-archas a supported catalog key.
Comment on lines
+91
to
+92
| | `arch`, `architecture`, `spec`, `specification`, `module`, `component`, `interface` | `arch` | | ||
| | `tc`, `test_case`, `test`, `verification_plan`, `vplan` | `tc` | |
| | `safety_v` | `hazard`, `safety_goal`, `fsr` | | ||
| | `sys` | `sysreq`, `sys-arch` | | ||
| | `sw` | `swreq`, `swarch` | | ||
| | `component` | one of (`req`, `comp_req`, `gd_req`), plus `arch`, `tc`. (`fmea` is best-effort and may be absent — see Step 6 of the component layer.) | |
Comment on lines
+291
to
+295
| Step 5c — `pharaoh-arch-draft` with `target_level=arch` and the requirement's ID as parent. | ||
|
|
||
| Step 5d — `pharaoh-arch-review` on the architecture element. | ||
|
|
||
| Step 5e — `pharaoh-vplan-draft` with `target_level=tc` and the requirement's ID as parent. |
Comment on lines
+47
to
+49
| - `artefact-catalog.yaml` — read the link-relationship map: which link types carry the | ||
| "satisfies its parent" semantics for each artefact type. Defaults: `:satisfies:` for | ||
| requirements and architecture; `:verifies:` for test cases; `:decides:` for decisions. |
Comment on lines
+38
to
+44
| Default values are NOT redeclared in this skill. If a flag is absent from the project's `pharaoh.toml`, the skill treats it as the value declared in `pharaoh.toml.example` at the Pharaoh repo root. The example currently sets `(require_change_analysis=true, require_verification=true, require_mece_on_release=false)` and `codelinks.enabled=true`; for absent strictness the example sets `"advisory"`. To change the defaults this skill walks against, edit `pharaoh.toml.example` only — never reintroduce competing defaults here. | ||
|
|
||
| Edge cases: | ||
| - `pharaoh_toml_path` missing or unreadable → emit `overall: "error"` with `errors: ["pharaoh.toml unresolved: <path>"]` and no other keys. Callers branch on `overall` first. No ladder array is emitted on this path — the ladder is meaningful only when the file parsed. | ||
| - TOML parse error (syntax bad) → same `overall: "error"` shape with the parser message included. | ||
| - Keys present but with unexpected types (e.g. `require_verification = "yes"` as a string) → treat as their typed default (`false` for booleans, `"advisory"` for strictness) and add a note `"unexpected type for <key>; treated as default"` in `notes`. | ||
| - Entire `[pharaoh.workflow]` or `[pharaoh.codelinks]` section absent → every flag in that section resolves to its `false` default; no error. | ||
| - Keys present but with unexpected types (e.g. `require_verification = "yes"` as a string) → treat as the typed default declared in `pharaoh.toml.example` (`true`/`false`/`"advisory"` per the example) and add a note `"unexpected type for <key>; treated as default"` in `notes`. | ||
| - Entire `[pharaoh.workflow]` or `[pharaoh.codelinks]` section absent → every flag in that section resolves to its example default; no error. |
Comment on lines
+46
to
+48
| - **parent_link** (from user or inferred): ID of the parent the new artefact links to | ||
| via the catalog-declared link relation (`:satisfies:` for classical reqs and FSRs, | ||
| `:safety_goal_for:` / `:derives_from:` / similar for safety-V — read from the catalog). |
Comment on lines
+191
to
+193
| | `[[needs.types]]` declared **and** the source tree has at least one `.. <directive>::` block in any `.rst` file under the source dir **and** ≥10% of those needs carry a status from a "matured" set (`approved`, `closed`, `reviewed`, `passed`). | `steady-state` | | ||
| | `[[needs.types]]` declared **and** the source tree has at least one `.. <directive>::` block in any `.rst` file. (Active drafting; not enough matured-status needs to qualify as steady-state yet.) | `reverse-eng` | | ||
| | `[[needs.types]]` declared **but** no `.. <directive>::` blocks found in any `.rst` file. (Types declared, no needs authored.) | `greenfield` | |
| prompt: Check that the authored artefact addresses the substance of its parent | ||
| - label: Review Drafted Requirement | ||
| agent: pharaoh.req-review | ||
| prompt: Audit the drafted requirement against the ISO 26262 §6 axes |
Comment on lines
+28
to
+31
| - **target_level** (from user, default `tc`): the artefact-catalog type name to emit. Any | ||
| verification-plan / test-case type declared in `.pharaoh/project/artefact-catalog.yaml` | ||
| is accepted. The emitted directive uses `target_level` verbatim as the directive name; the | ||
| ID prefix is resolved from `id-conventions.yaml`'s `prefixes` map. |
|
|
||
| | User input (any case) | Resolves to | | ||
| |---|---| | ||
| | `req`, `requirement`, `gd_req`, `comp_req`, `sysreq`, `swreq`, `feat` | the catalog key matching the user's `target_type` exactly when present; otherwise the catalog key whose suffix matches the request and which appears first in `artefact-catalog.yaml`'s declaration order. If two or more catalog keys match equally, FAIL with a clear `ambiguous target_type` error and list the candidates so the caller can resolve | |
This was referenced May 5, 2026
ubmarco
pushed a commit
to bburda/sphinx-needs-demo
that referenced
this pull request
May 5, 2026
Pulls the agent and prompt template state from the upstream PR that addresses my consolidated feedback (useblocks/pharaoh#13). Adds the two missing user-entry agents (@pharaoh.author, @pharaoh.verify) so the matching slash commands stop dispatching to nothing, and picks up the description and gitignore-guidance fixes from the same upstream branch (covers useblocks/pharaoh#11 and useblocks#12 in one shot). Replaces the local hand-edits applied earlier in this PR for the five agent files I had patched (pharaoh.write-plan, pharaoh.toctree-emit, pharaoh.execute-plan, pharaoh.feat-file-map, pharaoh.setup) with the upstream versions, so the templates stay in sync with the plugin.
ubmarco
pushed a commit
to bburda/sphinx-needs-demo
that referenced
this pull request
May 7, 2026
Pulls the agent and prompt template state from the upstream PR that addresses my consolidated feedback (useblocks/pharaoh#13). Adds the two missing user-entry agents (@pharaoh.author, @pharaoh.verify) so the matching slash commands stop dispatching to nothing, and picks up the description and gitignore-guidance fixes from the same upstream branch (covers useblocks/pharaoh#11 and useblocks#12 in one shot). Replaces the local hand-edits applied earlier in this PR for the five agent files I had patched (pharaoh.write-plan, pharaoh.toctree-emit, pharaoh.execute-plan, pharaoh.feat-file-map, pharaoh.setup) with the upstream versions, so the templates stay in sync with the plugin.
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
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.
Addresses the in-scope sections of #13 in one PR. Sections 7 (chains_from vs handoffs vocabulary divergence) and 11 (Copilot agents have no Python runtime) remain out-of-scope; each is filed as its own follow-up.
Commit history walks the issue section-by-section. Roadmap of work landing here:
Draft until all sections land; then ready for review.