Skip to content

docs: sync living-doc glossary with canonical definitions and drop pageobject status - #40

Open
miroslavpojer wants to merge 30 commits into
masterfrom
docs/39-align-living-doc-canon
Open

miroslavpojer wants to merge 30 commits into
masterfrom
docs/39-align-living-doc-canon

Conversation

@miroslavpojer

@miroslavpojer miroslavpojer commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Overview

The toolkit's skills, schemas and shared references had drifted from the canonical living-doc
conventions, and nothing prevented them from drifting again. One of the two canon copies under
skills/shared/references/ was maintained by hand and pinned to a moving master link.

This branch collects the canon-alignment work under one review. It does three things:

Key Changes

Canon sync — both copies are now generated and CI-verified

  • One scripts/sync-living-doc-reference.sh replaces the glossary-only script. It generates both
    copies from a table of canon paths, and the fetch and link-rewrite logic live in exactly one place.
  • Each copy pins a 40-character SHA in a provenance header on line 1, and the prose "keep in sync"
    instruction is gone. test-scripts.yml re-runs the sync for every target and fails on any diff.
  • Both copies are re-synced from living-doc@86c756d (feat: 26 - implement skill test unit standards #36), which defines external_dependencies and
    adds feature_dependencies. The earlier pin, 2ac58fc (feat: /create-repository #34), brought in ## Notes on every entity,
    the reworded test_type note, and the split between authored and generated files.

How to change a Feature's dependencies (living-doc-update)

  • The new section "Change a Feature's dependencies" covers three cases: a dependency is added, dropped,
    or promoted to a Feature of its own. It links the canon definitions instead of restating them, and
    states that a Feature's feature_dependencies is derived and never authored.
  • The promotion is an eight-step cascade:
    • it routes Feature creation to living-doc-create-feature;
    • it scans every Feature's external_dependencies, matching names case-insensitively and asking
      the user about near misses;
    • it records the edge on the Functionality that makes the call, never on the Feature;
    • it never re-parents a Functionality, and it flags the Functionalities whose tests stubbed the
      dependency for test review;
    • it reports the new Feature as ORPHAN_FEATURE, derived planned, without inventing placeholder
      Functionalities.
  • A dropped dependency triggers a catalog scan. Test doubles, fixtures or manifest.json entries that
    are now unreferenced are listed, not deleted.
  • The skill gets dependency triggers and a row in the change-type table. ## Update Feature ownership
    now stands on its own heading, and its text is unchanged.
  • living-doc-create-functionality asks for feature_dependencies and emits it.
    living-doc-create-feature sends a target that is already a documented API Feature to the calling
    Functionality instead of to external_dependencies.

validate_entity.py checks feature_dependencies edges

With --catalog, every target is checked against the same four rules living-doc's examples_check
enforces on the canon corpus:

Rule Severity Why
The target exists warning Canon expects an unresolved target in a project documented in source code
The target is not the Functionality's own Feature error Canon rules this edge invalid
The target is an API Feature error Canon rules this edge invalid
The target is not deprecated warning Derived from the target's Functionalities; points at superseded_by when there is one

Without --catalog, none of the four is reported; the validator only says the basis is unavailable.
Two checks work without a catalog:

  • the value must be a list of FEAT- ids (error);
  • a Feature must not author the field (warning).

This reverses an earlier decision. Please do not re-revert it. An earlier, superseded draft of this
work concluded that the validator should stay out of dependencies. That reasoning was about
external_dependencies: free-form names that resolve to nothing and can only be checked by
inventing meaning. It does not apply to a list of FEAT- ids, which the validator can resolve
against the catalog it already accepts, and whose surface type and lifecycle it can check.
external_dependencies stays unchecked, for the original reason.

Functionality entity now matches what canon authors

  • One name for the owning Feature: parent_feature. It corresponds to canon's # parent: header
    key and ## Parent Feature issue heading. living-doc-create-functionality, the rename cascades
    and their evals said feature_id, while the validator requires parent_feature, so every
    Functionality the skill drafted failed validation. feature_id keeps its canon meaning in
    seed.yaml, manifest.json and feature_registry.
  • Canonical name form. A Functionality is named <Feature name> - <verb phrase>, joined by a
    plain hyphen (e.g. Login Page - Validate Password Strength). The skill emitted the bare verb phrase,
    which the validator warns on. The verb-phrase rules still apply to the part after the hyphen.
    living-doc-scenario-creator uses this name as-is on the Feature: line, so the Feature name is
    never written twice.
  • Acceptance criteria are objects with IDs. Each AC is now {"id": "AC:<Functionality id>-<nn>", "description": …}, the same ID it carries in the header or issue body, and test_coverage
    references exactly those IDs. The skill emitted bare strings, which have no ID, and
    validate_entity.py crashed on them with AttributeError. The skill's "validate before saving"
    step therefore never ran. The validator now reports a bare-string AC as an error, with an example of
    the object form, for Functionalities and User Stories alike.
  • Also brought in line with canon:
    • exactly one parent Feature, where the skill said "at least one";
    • numeric Feature ids in the examples, replacing slug ids that the skill's own rules forbid;
    • living-doc-pageobject-scan puts the Feature ID in the header title line, not in a feature_id:
      field.

Agent and skill routing

  • living-doc-bdd-copilot.agent.md:
    • no longer tells the agent to author deprecated_at on a Feature;
    • a deprecated AC carries only its removal note;
    • dependency changes route to living-doc-update.
  • Descriptions. living-doc-update, living-doc-create-functionality and
    living-doc-create-feature were over the 1024-character limit in CONTRIBUTING.md. They are now 989,
    981 and 995 characters. Duplicate trigger phrases were merged; no capability or routing target was
    dropped.
  • Trigger overlap between living-doc-create-feature and living-doc-update. Both claimed renames,
    registry updates and dependency changes. Now living-doc-create-feature owns a Feature that does not
    exist yet, and any change to an existing Feature goes to living-doc-update: owners, dependencies,
    rename, registry entry, deprecation.
    • living-doc-create-feature's triggers are narrowed to match.
    • The trigger eval that expected create-feature to fire on a rename now expects it not to fire, and
      two negative evals cover a dropped and a promoted dependency.

func_type is now required on a Functionality

  • Canon marks it required, but the skill emitted entities without it. Such an entity is unusable
    unless someone invents the value. func_type is now in the template, every worked example,
    REQUIRED_FIELDS, and the evals.
  • validate_entity.py rejects a value outside the seven canonical ones.
  • A new "Choosing func_type" decision aid links the values table rather than duplicating it.

deprecated_code_commit replaced by a ## Notes bullet

  • The field was in neither the canon nor the contract. The information is kept: deprecated is not
    removed
    , and the commit is a pointer to where the removal happened, which is context, not schema.
  • Removed from the skill docs, the validator and its tests. The regression test was updated, not
    deleted.

Validator no longer claims a Feature state it cannot know

  • A Feature has no status; its state is derived from its Functionalities, which a single entity does
    not contain. The validator now derives it from --catalog and stays silent otherwise. When the basis
    is an authored marker rather than a derived state, it says so explicitly.
  • This follows from the canon: a Feature's deprecated_at is derived, never authored. The old code
    warned when it was missing, which told agents to write a generated field. Skill docs and evals are
    updated to match.
  • test_coverage is labelled as toolkit-local test planning, never authored into documentation.

Earlier alignment work on this branch

  • A single hyphen replaces en and em dashes across AC headers, bullets and entity names.
  • Descoping an AC emits (planned) / (v<target> - planned) with a - Rationale: bullet instead of
    status: descoped.
  • ac_states in project-profile.schema.json is constrained to the four canonical states, and
    pageobject_statuses is removed.
  • Feature/PageObject status writes are dropped from living-doc-create-feature,
    data-cy-instrument and related skills.
  • Eval fixtures are updated across the affected skills.

Evals added or updated in this round

  • living-doc-update:
    • happy-path eval 17 uses the new fixture dependency-promotion-catalog.json. In it, FEAT-001 and
      FEAT-002 both name payment-events, in different casing, and the prompt mentions only FEAT-001;
    • trigger entries t26–t28;
    • fixture-map.md is rebuilt from evals.json, which also fixes missing rows and wrong categories.
  • living-doc-create-functionality: evals 1, 6, 8, 9 and 10 expect the canonical name and AC object
    forms.
  • living-doc-create-feature: the trigger evals match the creation/change split, and the
    fixture-map.md summary is corrected from 18 entries to 29.

Known gaps

  • An authored ## Notes bullet is dropped until living-doc-utilities parses ## Notes.
  • No skill writes an entity into a GitHub issue body or an Azure DevOps work item. The create skills
    emit a toolkit JSON draft, and only feature-file headers are rendered: by
    living-doc-scenario-creator, and by living-doc-pageobject-scan for UI Features. Per canon, an
    API Feature can only be documented in an issue-backed project, so no skill can author a
    feature_dependencies target today. It needs a decision on rendering versus a per-project Storage
    Profile template, and is left for a follow-up.

Tests

All 13 test_*.py scripts under skills/ pass. They run as scripts, not under pytest; that is by
design, and new tests are registered in the file's __main__ runner.
test_validate_entity.py gains nine tests:

  • eight for feature_dependencies, covering each of the four catalog rules, the no-catalog basis,
    malformed values, a Feature authoring the field, and a valid edge;
  • one for bare-string ACs.
$ python3 skills/living-doc-update/scripts/test_validate_entity.py
✓ All tests passed!
  • Sync check. Earlier on this branch, the check was shown to fail as intended: a one-character
    edit to either canon copy fails the job, and so does a header re-pinned from a SHA to a branch name.
    Re-running the sync against 86c756d reproduces both committed copies with no diff.
  • Worked examples. Both real examples in living-doc-create-functionality (FUNC-001,
    FUNC-003) pass validate_entity.py with no issues, and their test_coverage matches their AC ids
    exactly.

Related

Closes #39

miroslavpojer and others added 4 commits September 15, 2026 14:01
…rofile schema to remove pageobject_statuses, and add regression tests for schema constraints
…tus field)

- Changed 'changing a Feature's ownership or status' to 'changing a Feature's ownership or deprecation metadata'
- Aligns front matter with body schema which correctly removed Feature.status field
- Features now only support deprecation metadata, not status lifecycle
- Fixes regression where skill incorrectly triggered on Feature status-change requests
- Resolves eval #3 failure in living-doc-update

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Changed guidance from 'update file header Acceptance Criteria block' to 'update # AC: comment directly above scenario'
- Resolves ambiguity about where stale AC description text should be updated
- Canonical rule: update the scenario-level # AC: comment (not file-level header block)
- File-level header is supplementary reference metadata, not the sync target
- Aligns skill behavior with eval #2 expectations (67% → 100% pass rate)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Several evals still require unparseable AC comments, while schema gating and statusless Feature deprecation handling remain incomplete.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Aligns living-documentation generation, validation, and guidance with canonical living-doc conventions while adding glossary drift prevention.

Changes:

  • Adds pinned glossary synchronization and CI drift checks.
  • Removes Feature/PageObject statuses and constrains canonical AC states.
  • Updates descope behavior, dash formatting, validators, documentation, and evals.
File summaries
File Description
skills/shared/references/schemas/test_project_profile_schema.py Adds schema regression checks.
skills/shared/references/schemas/project-profile.schema.json Constrains AC states and rejects PageObject statuses.
skills/shared/references/living-doc-glossary.md Syncs the canonical glossary.
skills/shared/references/living-doc-bdd-schemas.md Updates canonical BDD schemas.
skills/living-doc-update/SKILL.md Revises status, deprecation, and descope guidance.
skills/living-doc-update/scripts/validate_entity.py Validates statusless Features and orphan conditions.
skills/living-doc-update/scripts/test_validate_entity.py Tests revised Feature validation.
skills/living-doc-update/evals/fixture-map.md Updates descope fixture mapping.
skills/living-doc-update/evals/evals.json Revises update expectations.
skills/living-doc-scenario-creator/SKILL.md Canonicalizes scenario formatting.
skills/living-doc-scenario-creator/evals/evals.json Updates scenario expectations.
skills/living-doc-pageobject-scan/SKILL.md Removes PageObject status generation.
skills/living-doc-impact-analysis/SKILL.md Canonicalizes AC separators.
skills/living-doc-create-feature/SKILL.md Removes authored Feature status.
skills/living-doc-create-feature/evals/evals.json Updates Feature creation expectations.
skills/gherkin-living-doc-sync/SKILL.md Revises traceability and descope synchronization.
skills/gherkin-living-doc-sync/evals/fixture-map.md Updates sync fixture mapping.
skills/gherkin-living-doc-sync/evals/evals.json Revises sync expectations.
skills/data-cy-instrument/SKILL.md Uses stub-reason removal for instrumentation.
scripts/sync-living-doc-glossary.sh Adds pinned glossary synchronization.
docs/guides/living-doc-update.md Documents canonical status transitions.
docs/guides/gherkin-living-doc-sync.md Documents revised scenario tagging.
docs/guides/agent-design.md Updates traceability format.
.github/workflows/test-scripts.yml Adds drift checking and schema dependency.
.github/copilot-instructions.md Documents standard BDD artifact paths.
.github/agents/living-doc-bdd-copilot.agent.md Updates orchestration conventions.
.github/agents/evals/living-doc-bdd-copilot/evals.json Aligns agent expectations.
Review details

Suppressed comments (1)

.github/agents/evals/living-doc-bdd-copilot/evals.json:317

  • The expected traceability comment has a space after AC:, so scan_ac_links.py:52 will not recognize it. Require the same # AC:US-... form documented by the loaded scenario-creator skill.
      "expected_output": "Agent loads the `living-doc-scenario-creator` skill. Generates a .feature file for US-007 with 3 scenarios - one per active AC. The planned AC is skipped (not generated until active). Each scenario is preceded by a '# AC: US-007-0n (v1.0.0 - active)' traceability comment. Merge policy: if a scenario already exists for an AC, applies the 4-row decision table (skip if intent matches, update if GWT is stale, propose replacement if deprecated, flag if multiple scenarios exist per AC).",
  • Files reviewed: 27/27 changed files
  • Comments generated: 14
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/agents/evals/living-doc-bdd-copilot/evals.json Outdated
Comment thread .github/agents/living-doc-bdd-copilot.agent.md Outdated
Comment thread .github/workflows/test-scripts.yml
Comment thread skills/gherkin-living-doc-sync/SKILL.md Outdated
Comment thread skills/gherkin-living-doc-sync/evals/evals.json Outdated
Comment thread skills/living-doc-scenario-creator/SKILL.md
Comment thread skills/living-doc-scenario-creator/evals/evals.json Outdated
Comment thread skills/living-doc-scenario-creator/evals/evals.json Outdated
Comment thread skills/living-doc-update/SKILL.md
Comment thread skills/living-doc-update/scripts/validate_entity.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Canonical contract mismatches remain in Feature validation, AC synchronization, and generated header formats.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (4) — in code that hasn't changed since the last review.

scripts/sync-living-doc-glossary.sh:39

  • This external fetch has no connection or total timeout. Because the script is now a required CI gate, a stalled raw GitHub connection can occupy the job until GitHub's multi-hour job timeout instead of failing promptly. Add bounded --connect-timeout/--max-time values (and a small retry count) to keep the gate reliable.
    skills/gherkin-living-doc-sync/SKILL.md:167
  • Explicitly skipping the file-level Acceptance Criteria block leaves two conflicting copies after an AC version, state, or description changes. That header is not merely decorative: skills/shared/references/living-doc-bdd-schemas.md:79-81,99-111 says it holds mined US metadata. Update both the mined header entry and scenario-level comments in the same sync action, or remove the duplicated header contract; otherwise collectors can continue reading stale AC data.
    skills/gherkin-living-doc-sync/evals/evals.json:155
  • The expected repair replaces the required AC description with the rationale. Canonical # AC: comments contain the AC header followed by its description; the descope rationale should be an additional comment. Otherwise this eval rewards output whose scenario-level description no longer matches the catalog AC.
    skills/shared/references/living-doc-bdd-schemas.md:320
  • This newly re-added Functionality header still combines the entity ID/middle-dot form with an em dash, contrary to the PR's explicit single-hyphen acceptance criterion. Use a plain hyphen in the structural header too.

skills/living-doc-update/scripts/validate_entity.py:195

  • ORPHAN_FEATURE is emitted only when both link lists are empty, but gap-finder defines an orphan as any Feature with no linked User Story (skills/living-doc-gap-finder/scripts/compute_gaps.py:152-163). A Feature with Functionalities but no User Stories therefore gets a generic user_stories warning instead of the distinct condition promised by this change. Emit ORPHAN_FEATURE whenever no_user_stories is true, and report an empty Functionality list separately.
        if no_user_stories and no_functionalities:
  • Files reviewed: 30/30 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread skills/living-doc-update/scripts/validate_entity.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Validator false positives, inconsistent documented contracts, and stale eval expectations remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (4) — in code that hasn't changed since the last review.

scripts/sync-living-doc-glossary.sh:59

  • REF is interpolated directly into a sed replacement, so valid refs containing replacement metacharacters are corrupted. For example, release&docs expands & to the matched text and produces a broken link instead of a URL containing the literal ref. Either restrict the documented input to commit SHAs or URL-encode the ref and escape sed replacement characters before rewriting links.
    skills/shared/references/living-doc-bdd-schemas.md:320
  • This generated header still combines an em dash and middle dot with a FUNC- ID, contrary to issue #39's acceptance criterion requiring plain hyphens for these entity headers. As written, the newly restored template continues producing the noncanonical form this PR is intended to remove.
    .github/agents/evals/living-doc-bdd-copilot/evals.json:274
  • This revised fixture still conflicts with the skill it loads. living-doc-create-feature/SKILL.md:129-137,164-172 does not define route as a Feature-entity field, and lines 41-45 say not to leave both relationship arrays empty unless the prompt explicitly says the links are unknown. This prompt does not; requiring route plus empty arrays rewards output outside the skill's canonical shape. Remove route and expect provisional links or explicit link discovery instead.
    docs/guides/living-doc-update.md:14
  • The guide now defines the canonical status progression here, but its Testing Evals section at line 69 still says draft → ready → deprecated. That leaves the same document advertising two incompatible state vocabularies; update the summary to planned → in_review → active → deprecated (the natural-language trigger “mark user story ready” can remain).

skills/living-doc-update/scripts/validate_entity.py:197

  • The new orphan check only inspects the Feature's own user_stories list. When --catalog is supplied and a User Story links this Feature through its features list, compute_gaps.py correctly treats it as linked, but this validator still emits ORPHAN_FEATURE. Account for the reverse catalog link here and add the equivalent regression case from living-doc-gap-finder/scripts/test_compute_gaps.py:23-35.
        no_user_stories = isinstance(entity.get("user_stories"), list) and not entity["user_stories"]
        no_functionalities = isinstance(entity.get("functionalities"), list) and not entity["functionalities"]
        # gap-finder's ORPHAN_FEATURE gap fires on the absence of a linked User Story alone
        # (compute_gaps.py), regardless of Functionality links — so mirror that here rather
        # than requiring both lists to be empty.
  • Files reviewed: 32/32 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread skills/living-doc-update/scripts/validate_entity.py Outdated
Comment thread skills/shared/references/living-doc-bdd-schemas.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow permits shell injection, and deprecated-Functionality cleanup and profile-narrowing coverage remain incomplete.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

skills/bdd-maintain/SKILL.md:120

  • The deprecated-entity rule omits a directly deprecated Functionality, even though Functionality feature files own their own scenarios and Functionalities are one of the two entity types that carry status. As written, cleanup guidance covers a deprecated User Story and a derived deprecated Feature, but not status: deprecated on a Functionality.
    skills/shared/references/living-doc-bdd-schemas.md:320
  • This generated Functionality header still combines a FUNC- ID and middle dot with an em dash, contrary to issue #39's explicit dash acceptance criterion. Replace the leading em dash with the canonical plain hyphen so this template does not regenerate the format the PR is removing.
  • Files reviewed: 32/32 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread .github/workflows/test-scripts.yml Outdated
Comment thread skills/living-doc-update/scripts/test_validate_entity.py Outdated
…rence and enhance validation test cases for acceptance criteria

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Required entity headers remain incomplete, profile narrowing is inconsistent, and arbitrary glossary refs are not safely URL-encoded.

Review details

Suppressed comments (6)

Previously missed (5) — in code that hasn't changed since the last review.

.github/agents/evals/living-doc-bdd-copilot/evals.json:37

  • This orchestrator eval checks only the human-readable # AC: comment, so output with no machine-readable @AC: tag can pass even though the agent contract requires both for every scenario (.github/agents/living-doc-bdd-copilot.agent.md:281-285). Add the matching @AC:US-007-0n tag to the expected output and expectations.
    scripts/sync-living-doc-glossary.sh:33
  • The script advertises support for any branch or tag, but interpolates the ref into a URL without encoding it. Valid ref characters such as # are URL delimiters (release#docs makes curl request only /release), so those advertised refs cannot be fetched. Percent-encode the ref while preserving /, or restrict and validate the API to commit SHAs.
    skills/living-doc-scenario-creator/SKILL.md:128
  • This copy-verbatim # Acceptance Criteria: header still uses the scenario-comment form (AC ... - description) and omits the required authored User Story # status: field. The canonical header schema in skills/shared/references/living-doc-bdd-schemas.md:86-112 puts each description on its own # - ... line; generated files following this example can therefore be mined with malformed or incomplete metadata. Replace this header fragment with the canonical User Story header shape.

This issue also appears on line 161 of the same file.
skills/living-doc-scenario-creator/evals/evals.json:287

  • This regression expectation explicitly requires the generated Functionality file to start at @FUNC_ID, so it rewards omission of the mandatory Functionality comment header. Require the canonical header (including status, parent, func_type, and acceptance_criteria) before @FUNC_ID; otherwise this eval can pass output that entity mode and living-doc-bdd-schemas.md:313-360 reject.
    skills/living-doc-update/scripts/validate_entity.py:411
  • --profile narrows only VALID_AC_STATUSES; User Story and Functionality entity status values are still checked against the full VALID_STATUSES set. Thus a profile with ac_states: [planned, active] incorrectly accepts an entity with status: in_review, despite the glossary defining authored entity statuses from that profile vocabulary. Apply the validated subset to both status sets and cover the entity-status path.

skills/living-doc-scenario-creator/SKILL.md:161

  • Entity mode promises a full Functionality feature-file header, but this example begins at @FUNC_ID and omits the required metadata/acceptance-criteria comment block defined in living-doc-bdd-schemas.md:313-360. Since this is the skill's generation example, following it produces a file without the mined Functionality metadata. Include the canonical Functionality header before the tag.
Feature: Login Page - Validate Password Strength
  • Files reviewed: 32/32 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

…d URL encoding in sync script, and improve validation tests for entity status

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

User Story deprecation guidance and several affected eval fixtures remain inconsistent with the canonical contracts.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

skills/living-doc-update/SKILL.md:110

  • This deprecation table now excludes User Stories even though they also carry authored status and deprecation metadata (as the decision table at line 36 states). Following this workflow for a User Story would leave it active while treating it as deprecated; include User Stories in the heading and applicable rows.
    skills/shared/references/living-doc-bdd-schemas.md:72
  • The affected gap-finder fixture was not aligned: skills/living-doc-gap-finder/evals/files/catalog-snapshot.json:5-7 still gives User Stories the noncanonical ready status. This contradicts the four-state contract introduced here and the PR's requirement to update every affected gap-finder fixture; use the appropriate canonical state and adjust dependent expectations if needed.
    skills/shared/references/living-doc-bdd-schemas.md:264
  • The documented manifest contract now uses test_id, suggested_test_id, and a string navigation_context, but skills/living-doc-pageobject-scan/evals/evals.json:191-197 still requires data-cy/suggestedDataCy, while .github/agents/evals/living-doc-bdd-copilot/evals.json:164-170 requires data_cy and an object. Those evals will penalize canonical output, so update both as part of the promised fixture synchronization.
  • Files reviewed: 32/32 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Profile validation can still silently bypass constraints, and several skill and eval contracts remain inconsistent.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

Previously missed (5) — in code that hasn't changed since the last review.

skills/data-cy-instrument/SKILL.md:270

  • Removing stub-reason: only proves locator instrumentation is complete; it does not satisfy the Functionality promotion gate. living-doc-update/SKILL.md:71-77 requires every AC to have passing tests, so this unconditional routing can mark behavior active before it is verified. Preserve planned unless that gate also passes.
    skills/living-doc-create-feature/SKILL.md:160
  • This worked pattern handles only annotated REST/GraphQL endpoints, so a purely asynchronous service with an AsyncAPI-annotated event handler is incorrectly rejected. That contradicts the canonical branch documented above (lines 44-56) and evals 6/12; include annotated event handlers in the API path and reject only when neither anchor exists.
    skills/living-doc-update/scripts/validate_entity.py:117
  • An explicitly supplied but missing/unreadable --profile only warns and then validates against the unrestricted canonical defaults. That silently bypasses the profile narrowing this change is trying to enforce; fail the command instead, just as for malformed YAML or missing PyYAML.

This issue also appears on line 124 of the same file.
skills/data-cy-instrument/evals/evals.json:145

  • This is now the twelfth eval, but docs/guides/data-cy-instrument.md:57 still documents 11 test cases. Update the guide count with this addition.
    skills/living-doc-scenario-creator/evals/evals.json:286
  • Adding eval 22 makes the guide's test-count statement stale: docs/guides/living-doc-scenario-creator.md:62 still says this skill has 21 test cases. Update it to 22 so the published guide matches the eval suite.

skills/living-doc-update/scripts/validate_entity.py:130

  • This type check still accepts falsey non-mapping YAML roots: yaml.safe_load(f) or {} converts [], false, 0, and "" to {} before reaching it. Preserve the loaded value and default only None so every non-mapping root is rejected as intended.
  • Files reviewed: 38/38 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread .github/agents/evals/living-doc-bdd-copilot/evals.json

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Profile-excluded AC states still produce exit code 0 and valid: true, so the validator does not enforce the configured vocabulary.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 40/40 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread skills/living-doc-update/scripts/validate_entity.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Generated entity contracts remain inconsistent with validator and eval behavior, and Feature deprecation validation misses empty markers.

Review details

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

skills/living-doc-create-feature/SKILL.md:124

  • The output contract still permits fabricated relationship IDs: lines 41 and 99-104 instruct the skill to invent values such as US-checkout and FUNC-001, while the updated orchestrator eval requires empty arrays when no catalog-backed relationships were supplied. Following the skill will therefore fail its own eval and can create dangling references. Make relationship IDs catalog-backed only; otherwise emit [] with the existing warning.
    skills/living-doc-update/SKILL.md:37
  • The updated deprecation contract is not reflected by eval 3 in skills/living-doc-update/evals/evals.json, which still universally requires status: deprecated for LegacyPaymentGatewayService. That entity previously represented a Service Feature, so the eval can reward the Feature status this row now forbids. Make the eval's entity type explicit and require Feature metadata/owned-Functionality deprecation when it is a Feature.
    skills/living-doc-update/scripts/test_validate_entity.py:656
  • This new alias test does not use the shape produced by living-doc-create-user-story, which emits "type": "UserStory" and "title". validate_entity.py currently rejects that generated artifact immediately as an unknown entity type, while this test passes with the validator-specific "User Story"/"name" spelling and gives false integration confidence. Normalize the real generated shape in the validator (or change the generator) and test that exact contract.
    skills/living-doc-update/scripts/validate_entity.py:226
  • Feature deprecation is detected by value truthiness rather than marker presence. A Feature containing "deprecated_at": "" (or another empty deprecation marker) is therefore treated as not deprecated and receives no warning for its missing audit metadata. Detect whether any marker key is present so empty values are reported by the validation loop below.
  • Files reviewed: 40/40 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Feature creation, relationship generation, deprecation detection, and fixture mapping still contain contradictory or incomplete behavior.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

skills/living-doc-scenario-creator/evals/fixture-map.md:22

  • The map still jumps from eval 11 directly to eval 22, leaving evals 12–21 undocumented even though several of those fixtures are updated by this PR. This does not satisfy the linked issue’s requirement that affected fixture maps match the eval suite. Add mapping rows for IDs 12 through 21.

skills/living-doc-update/scripts/validate_entity.py:228

  • Feature deprecation can also be signaled by deprecated_code_commit, which the updated workflow explicitly allows for Features (SKILL.md:110). Because that marker is absent here, a Feature containing only deprecated_code_commit is treated as not deprecated and receives no warnings for missing deprecated_at or deprecation_reason. Include it in the marker set.
  • Files reviewed: 42/43 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread skills/living-doc-create-feature/SKILL.md Outdated
Comment thread skills/living-doc-create-feature/SKILL.md
Comment thread skills/living-doc-create-feature/evals/evals.json Outdated
Comment thread skills/living-doc-create-user-story/SKILL.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Conflicting creation and promotion instructions still permit fabricated links, circular Feature creation, and skipped validation gates.

Review details

Suppressed comments (7)

Previously missed (3) — in code that hasn't changed since the last review.

skills/data-cy-instrument/SKILL.md:213

  • The new promotion gate is not propagated to the instructions/evals that drive behavior: line 224 still says to change the status after Instrument and Sync, and eval 5 (evals/evals.json:57-62) explicitly expects that unconditional promotion. The eval will therefore reward bypassing the required coverage and passing-test checks. Update both to require living-doc-update to verify its full promotion gate before changing status.
    skills/living-doc-create-user-story/evals/files/incomplete-user-story.json:5
  • This only partially migrates the fixture: the canonical User Story shape now documented in SKILL.md:144-176 uses top-level as_a, i_want, and so_that, but this file still nests as_a/i_can under narrative; eval 2 then explicitly says not to flag the narrative. The eval will preserve a legacy shape while claiming to test a canonical User Story. Flatten these fields and use i_want.
    docs/guides/living-doc-create-feature.md:44
  • This repeats the circular requirement from the skill: a planned UI surface cannot have its PageObject until the scan proceeds, while the orchestrator requires creating its Feature first. Describe the PageObject as the eventual UI test abstraction rather than a prerequisite for documenting the Feature.

skills/living-doc-create-feature/SKILL.md:49

  • Making the test abstraction pre-exist the Feature creates a circular creation path: .github/agents/living-doc-bdd-copilot.agent.md:187 routes an unlinked scanned route through Feature creation before continuing, and eval 8 creates a UI Feature without supplying a PageObject. Treat the PageObject as the eventual UI abstraction; only reject components that are neither UI surfaces nor documented API contracts.
Select the surface type — only two exist, and each requires the matching test abstraction to actually exist for this surface:

skills/living-doc-create-feature/SKILL.md:123

  • The new no-fabrication rule is still contradicted by this skill's required literal example (user_stories: ["US-example"], functionalities: ["FUNC-001"] at lines 147-148) and by eval 1, which still expects inferred links without catalog evidence (evals/evals.json:8). Agents following those instructions will emit dangling IDs; update both examples/evals to use empty arrays unless the catalog confirms the links.
Output the entity as a **single fenced `json` code block** whenever you have enough information to draft it. The block must contain **only** the JSON object — no prose, no bullets, no warnings inside the fence. The literal first line of the block must be ````json` and the closing line must be ``` . Code fences are required plain text, not optional formatting. Keep any warnings or follow-up questions **outside** the code block. If the user gives a named surface but not all metadata, ask the missing questions and still include a starter draft in the same reply, using inferred purpose/surface type/external_dependencies, and `[]` for `user_stories`/`functionalities` unless the catalog already backs the link. A Feature never carries a `status` field — its state is derived from its Functionalities, never authored. If the request explicitly asks to create the entity from the given details, emit the draft immediately.

skills/living-doc-create-feature/evals/evals.json:71

  • This prompt supplies no catalog relationship, but the expected output still requires at least one User Story link. That conflicts with the new rule to never fabricate user_stories or functionalities, so the eval rewards dangling IDs. Require empty arrays unless catalog evidence is provided.
      "expected_output": "Agent identifies this as a Feature entity creation (system surface). Asks: only UI and API exist — does it expose a REST/GraphQL endpoint with an annotated endpoint method, or a message-broker topic (e.g. Kafka) with an AsyncAPI (or equivalent) annotation on the producer/consumer handler (either is its API contract anchor), or is it purely topic/queue-driven with no such annotation at all? What User Stories does it enable? What Functionalities does it own? Who are the owners? What are the external dependencies (SMTP relay, template store)? If it exposes an annotated endpoint or an annotated event handler, runs next_id.py and outputs a canonical Feature JSON with id=FEAT-001 (or next numeric ID), surface_type=API documenting that contract, at least one User Story link, owners, and external_dependencies. If it is purely topic/queue-driven with no annotation at all, explains it has no UI/API test-abstraction anchor yet and should instead be recorded as an external_dependencies entry on the Features that use it, rather than created as a Feature itself.",

skills/living-doc-create-user-story/SKILL.md:211

  • This generic path still hardcodes FEAT-001, contradicting the catalog-only rule at line 30. In repositories where that ID is absent or belongs to another surface, the generated story gains a false relationship; use the catalog's Login Feature ID or the [NEW: Login] marker.
For a generic password-reset prompt with no channel specified, default the starter draft to `name: "Reset password"`, `features: ["FEAT-001"]`, and still emit the JSON immediately after the four questions.
  • Files reviewed: 42/43 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@miroslavpojer
miroslavpojer marked this pull request as ready for review September 17, 2026 12:10
miroslavpojer and others added 6 commits September 17, 2026 14:15
skills/shared/references/ held two canon copies under two regimes: the glossary was
script-generated, SHA-pinned and CI-verified, while living-doc-bdd-schemas.md was
hand-maintained, pinned to a moving master link, and guarded only by the prose
instruction "Keep both in sync when either changes". It had drifted to 600 lines
against the canon's 677.

Generalise sync-living-doc-glossary.sh into sync-living-doc-reference.sh, driven by a
table of (target -> canon path -> local path) pairs. The fetch, ref percent-encoding
and link-rewrite logic now exist in exactly one place; no code is copied into a second
script. Uses a plain indexed array rather than declare -A, since macOS ships bash 3.2.

Both copies carry a line-1 provenance header pinning a 40-character SHA and a
"generated file" line; the prose keep-in-sync instruction is gone. test-scripts.yml's
glossary-sync-check becomes canon-sync-check and loops over every target the script
declares, asserting a provenance header exists, the pinned ref is 40 hex, and a
re-run produces no diff.

Re-sync both copies from merged P35-LD7c canon (living-doc@2ac58fc). The glossary was
still pinned to 6bbfd949 and its Feature-deprecation semantics are needed downstream.
The re-sync restores the authored-vs-tooling-file intro split, adds Repository
adoption scope and Maintaining a PageObject header, brings notes:/## Notes onto every
entity, lists deprecated_at: on a PageObject header as a mistake, and carries the
reworded test_type sentence with no hand edit anywhere.

Toolkit-local content with no canon equivalent (the schemas/ pointer) is emitted by
the script's local_addendum function, so it stays reproducible under CI's diff.

Per DEC-4, the three format sections P35-AG1 was to "add if missing" -- the User
Story, Feature (PageObject) and Functionality file headers -- all live in
living-doc-bdd-schemas.md, which this change puts under generated, SHA-pinned,
CI-verified sync.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

agentic-toolkit: align with the living-doc canon

2 participants