fix(pharaoh-setup): infer required_links direction from edges, not link names#14
Closed
fix(pharaoh-setup): infer required_links direction from edges, not link names#14
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
There was a problem hiding this comment.
Pull request overview
Updates pharaoh-setup’s Step 2b documentation so [pharaoh.traceability].required_links direction is inferred from observed link edges (preferably via built needs.json) rather than guessing from link option names, avoiding inverted chains on projects with opposite link conventions.
Changes:
- Replaces the heuristic link-name→chain-direction table with a needs.json–driven inference algorithm (sample-size + coverage thresholds).
- Clarifies
required_linkssemantics as “outgoing link from source-type to target-type” perpharaoh:mece. - Adds a “refuse to guess” behavior: emit TODO comments when direction/type-pair can’t be determined.
Comment on lines
+283
to
+284
| # found. Build the docs once and re-run `pharaoh:setup`, or add an | ||
| # explicit chain manually in the form "source-type -> target-type". |
| 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`. |
Comment on lines
+273
to
+275
| **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. |
Comment on lines
+290
to
+296
| **Type-pair filter (applied to every source).** Emit a chain only when both the source type and the target type are declared in `ubproject.toml` `[[needs.types]]`. When Source 1 resolves an edge whose target type is not declared, drop it with a comment naming the dropped target — the chain is dead config that would alarm on every source need from day one: | ||
|
|
||
| ```toml | ||
| required_links = [ | ||
| "req -> spec", | ||
| "spec -> impl", | ||
| # "impl -> test", # SKIPPED: 'test' is not declared in [[needs.types]] |
bburda
added a commit
that referenced
this pull request
May 5, 2026
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.
patdhlk
pushed a commit
that referenced
this pull request
May 5, 2026
* fix(pharaoh-setup): infer required_links direction from edges, not link 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 * fix(agents): sync agent.md descriptions with SKILL.md, fix gitignore 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. * cleanup: drop dangling pharaoh-arch-regenerate references 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). * arch-review: fix grammar in advisory-chain reminder "Confirm the findings clear" parses as a sentence missing a verb; "confirm the findings are resolved" reads cleanly. Caught in PR review. * feat(skills): add pharaoh-author and pharaoh-verify user-entry skills 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). * schemas: promote tailoring schemas to canonical install path 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. * tailor: reconcile bootstrap/fill output shapes against canonical schemas 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. * release-gate: emit and validate required_links/metadata_fields/roles 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). * toml-example: document all consumed pharaoh.toml sections, single defaults 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). * atomic: parameterize pharaoh-arch-draft and pharaoh-vplan-draft by target_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). * atomic: extend pharaoh-req-draft to cover ISO 26262 safety-V types 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). * flow: extend pharaoh-flow to SYS/SWE split and safety V 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. * setup: read project state before imposing defaults 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. * fix: pass target_level to pharaoh-req-draft and populate score example 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. * fix(review): address 11 cross-phase review findings 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
Contributor
Author
|
#16 included this |
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.
Summary
pharaoh-setupStep 2b inferred[pharaoh.traceability].required_linkschain direction from a heuristic table mapping link option names to fixed chains (implements -> "spec -> impl",tests -> "impl -> test", etc.). The table assumed a single project convention (parent references child via the link option). Projects that follow the opposite convention -- child references parent via:implements:,:reqs:, etc. -- received chains pointing the wrong way, andpharaoh:mecereported 100% gaps for the source type.What changed
skills/pharaoh-setup/SKILL.mdStep 2b now resolves chain direction from three sources in priority order:needs.jsonwhen available. For each declared link optionLand ordered pair of declared types(X, Y), emit"X -> Y"only when at least 90% ofXinstances bearing:L:resolve to aY, with a sample size of at least 3. The direction matches the edges the project actually emits.[needs.links.<name>]. Used only whenubproject.tomlcarries an explicit type-pair hint; theoutgoing/incomingdescription alone does not identify the type pair, so the skill does not invent it from the link option name.(source-type, target-type)pair, emit a TODO comment instead of a chain.The previous heuristic-name table is removed. Type-pair filtering against
[[needs.types]]and the empty-array fallback continue to apply to every source.Repro / verification
Reproduction is documented in #11 against
useblocks/sphinx-needs-demo. After the fix, whenneeds.jsonis available, Step 2b emits chains that match the demo's actual link semantics (spec -> req,impl -> spec,safety_goal -> hazard,fsr -> safety_goal,arch -> req) andpharaoh:mecereports 0 gaps.Closes #11