Skip to content

fix(interview): clarify invocation contract + fix ok inconsistency + add setup reference tests#329

Merged
MScottAdams merged 2 commits intomasterfrom
agent2/fix/302-303-304-interview-cleanup
Apr 13, 2026
Merged

fix(interview): clarify invocation contract + fix ok inconsistency + add setup reference tests#329
MScottAdams merged 2 commits intomasterfrom
agent2/fix/302-303-304-interview-cleanup

Conversation

@MScottAdams
Copy link
Copy Markdown
Collaborator

Summary

Closes #302, #303, #304

Three targeted fixes to the deft-interview skill addressing P2 findings from PR #299:

  1. Clarify invocation contract (fix(skill): deft-interview invocation contract -- clarify embedded vs delegation usage modes #302, t1.27.1): Restructured the Invocation Contract section into two subsections -- Embedded Mode (calling skill references rules inline, no contract object, used by deft-setup) and Delegation Mode (explicit sub-skill invocation with formal contract object). The formal contract requirements (MUST provide) are now scoped to Delegation Mode only.

  2. Fix Rule 5 vs Rule 6 ok inconsistency (fix(skill): deft-interview Rule 5 vs Rule 6 inconsistency -- 'ok' accepted for default but not at confirmation gate #303, t1.28.1): Added clarifying note to Rule 6 confirmation gate explaining the intentional strictness asymmetry -- Rule 5 accepts casual ok for individual defaults (low cost, correctable at the confirmation gate), while Rule 6 requires explicit yes/confirmed/approve for the entire artifact (high cost, guards against auto-fill).

  3. Add regression tests for deft-setup Phase 1/2 deft-interview references (test(skill): add regression test for deft-setup Phase 1/2 referencing deft-interview #304, t1.29.1): Added 2 assertions to tests/content/test_skills.py verifying deft-setup Phase 1 and Phase 2 each reference deft-interview.

Related Issues

Closes #302, Closes #303, Closes #304

Checklist

  • /deft:change -- N/A (<3 file changes, documentation + test only)
  • CHANGELOG.md -- added entries under [Unreleased]
  • ROADMAP.md -- N/A (updated at release time per convention)
  • Tests pass locally (task check: 1055 passed, 2 xfailed)

…add setup reference tests

Closes #302, #303, #304

- Clarify embedded vs delegation invocation modes in deft-interview SKILL.md
- Resolve Rule 5 vs Rule 6 ok acceptance inconsistency
- Add test_skills.py assertions for deft-setup Phase 1/2 deft-interview references
@MScottAdams
Copy link
Copy Markdown
Collaborator Author

@greptileai review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 13, 2026

Greptile Summary

This PR delivers three focused documentation/test fixes to skills/deft-interview/SKILL.md and tests/content/test_skills.py, closing issues #302, #303, and #304. The changes restructure the Invocation Contract into Embedded vs Delegation modes, add a clarifying asymmetry note to Rule 6, and add regression tests verifying deft-setup Phase 1 and Phase 2 both reference deft-interview — all matching their SPECIFICATION acceptance criteria exactly.

Confidence Score: 5/5

Safe to merge — documentation and test-only changes with no logic or runtime risk.

All three changes are scoped to Markdown documentation and a Python test file. The SKILL.md edits are accurate and consistent with the existing RFC2119 style. The new tests follow the established str.find() section-slicing pattern used throughout the file, and the PR author confirms all 1055 tests pass. CHANGELOG and SPECIFICATION entries match the implemented changes exactly. No P0 or P1 findings.

No files require special attention.

Important Files Changed

Filename Overview
skills/deft-interview/SKILL.md Invocation Contract section restructured into Embedded/Delegation subsections; Rule 6 clarifying note added — both changes are accurate, well-scoped, and consistent with RFC2119 conventions used throughout the file.
tests/content/test_skills.py Two new regression tests added (section 31) that slice Phase 1 and Phase 2 text using str.find() to assert deft-interview is referenced in each phase; approach is consistent with the existing test patterns in this file.
CHANGELOG.md Adds accurate [Unreleased] Fixed entries for #302 and #303; Added entry for #304 regression tests; content matches the actual changes made.
SPECIFICATION.md Task statuses for t1.27.1, t1.28.1, and t1.29.1 flipped to [completed] with acceptance criteria that precisely match the implemented changes.

Sequence Diagram

sequenceDiagram
    participant CS as Calling Skill
    participant DI as deft-interview
    participant U as User

    Note over CS,U: Embedded Mode (e.g. deft-setup Phase 1/2)
    CS->>CS: References deft-interview rules inline
    CS->>U: Asks questions (applying Rule 1–7 internally)
    U-->>CS: Answers
    CS->>U: Confirmation gate (Rule 6)
    U-->>CS: yes / confirmed / approve
    CS->>CS: Generates artifact using captured answers

    Note over CS,U: Delegation Mode (explicit sub-skill invocation)
    CS->>DI: Invokes with contract object (required fields, question defs, optional fields)
    DI->>U: Asks questions per Rule 1–7
    U-->>DI: Answers
    DI->>U: Confirmation gate (Rule 6)
    U-->>DI: yes / confirmed / approve
    DI-->>CS: Returns structured answers map
    CS->>CS: Validates answers map and generates artifact
Loading

Reviews (2): Last reviewed commit: "fix: address Greptile review findings (b..." | Re-trigger Greptile

- Merge duplicate ### Fixed sections in CHANGELOG.md into single section

- Renumber test section comments from 30b to sequential 31/32/33
@MScottAdams MScottAdams merged commit f0084d1 into master Apr 13, 2026
9 checks passed
@MScottAdams MScottAdams deleted the agent2/fix/302-303-304-interview-cleanup branch April 13, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant