Skip to content

Restructure skills to align with skill-creator guidelines#2

Merged
leeovery merged 1 commit intomainfrom
claude/fix-skill-structure-013HwHkHPpPxMfpVy48BgTXp
Nov 23, 2025
Merged

Restructure skills to align with skill-creator guidelines#2
leeovery merged 1 commit intomainfrom
claude/fix-skill-structure-013HwHkHPpPxMfpVy48BgTXp

Conversation

@leeovery
Copy link
Copy Markdown
Owner

  • Organize supporting documentation into references/ directories
  • Rename technical-discussion/skill.md to SKILL.md (proper naming)
  • Update all internal references to use correct relative paths
  • Maintain all content without loss

Both skills now follow the proper structure:

  • SKILL.md (required main file)
  • references/ (supporting documentation)

This aligns with the skill-creator best practices for progressive
disclosure and proper file organization.

- Organize supporting documentation into references/ directories
- Rename technical-discussion/skill.md to SKILL.md (proper naming)
- Update all internal references to use correct relative paths
- Maintain all content without loss

Both skills now follow the proper structure:
- SKILL.md (required main file)
- references/ (supporting documentation)

This aligns with the skill-creator best practices for progressive
disclosure and proper file organization.
@leeovery leeovery merged commit 8629ca7 into main Nov 23, 2025
@leeovery leeovery deleted the claude/fix-skill-structure-013HwHkHPpPxMfpVy48BgTXp branch November 23, 2025 20:17
leeovery added a commit that referenced this pull request Apr 11, 2026
Addresses two gaps between the chunker implementation and the design
contract, surfaced during PR #244 review.

Issue #1 — merge-up broke content preservation (design line 74)
================================================================
The old merge-up path joined sections with a hard-coded `\n\n`:

    prev.content = prev.content + '\n\n' + section.text;

That separator is injected by the chunker, not drawn from the source.
If the gap between the parent and the merge-up section contained
anything other than exactly two newlines (e.g. a blank line with
trailing whitespace, or a comment), the merged chunk was no longer a
verbatim substring of the source — silently violating the global
"no lossy compression" invariant.

Fix: track source line ranges on every section during buildSections and
maintain them through the processing pipeline. merge-up now extends the
preceding segment's endLine; chunk content is generated by slicing from
the original line array once at the end. Contiguous source slice →
verbatim guaranteed, regardless of what sits between the merged
sections in the source.

Issue #3 — sub-level special_sections was not implemented
==========================================================
Task 2-1 states: "own-chunk: always split into its own chunk
regardless of heading level". The previous implementation only matched
at the split level (H2 for discussion), so an H3 "### Discussion Map"
nested inside a regular H2 would remain part of its parent chunk
instead of being extracted.

Fix: added expandSubLevelSpecials, which walks each split-level
section's line range and carves out any sub-level heading whose text
matches a special_sections entry. Precedence rule: if the parent
section's heading is itself in special_sections, the parent's action
wins and no sub-carving happens — "Discussion Map as H2" stays one
chunk. Sub-level matching handles own-chunk and skip; merge-up is a
split-level concept only and sub-level merge-up entries are treated
as regular sub-headings.

Issue #2 — own-chunk + size fallback ambiguity
==============================================
The task wording "always split into its own chunk" is ambiguous: does
own-chunk bypass the max_lines fallback split, or not? The previous
code made a silent choice. Added an inline comment documenting the
decision and its reasoning: own-chunk is interpreted as literal one
chunk, so a large Discussion Map stays intact even if it exceeds
max_lines. special_sections mark semantic units the user has declared
atomic, and splitting them would defeat that declaration.

Testing
=======
- Two new merge-up tests assert the verbatim invariant on
  synthetic sources with non-`\n\n` separators — would have caught the
  original bug. Both pass.
- One new sub-level extraction test using a synthetic fixture
  (`sub-level-special-fixture.md`) where `### Discussion Map` lives
  inside `## Plan`. Verifies the Map is extracted, the parent's intro
  stays, and "## Context"/"## Summary" boundaries are unaffected.
- One parent-wins precedence test against the real pigeon discussion
  fixture — makes the precedence rule explicit.

Additional real fixtures for structural diversity
==================================================
- spec-deep-nested-fixture.md (tick v1 tick-core/specification.md, 810L)
  — one huge H2 ("## Specification") that gets fallback-split into 11
  H3 chunks, plus one standalone H2 ("## Dependencies"). Proves the flat
  fallback chain handles real artifacts with a single dominant H2.
- research-oversized-h3-fixture.md (tick v1 research/exploration.md,
  567L) — contains a ~310-line H3 "Session 1" that stays intact (no
  recursion, reported by knowledge status in Phase 4).
- discussion-q-style-fixture.md (tick v1 cli-command-structure-ux.md,
  416L) — Q1..Q6 discussion shape with no Discussion Map, proves the
  chunker copes with non-canonical discussion variants.
- spec-folio-fixture.md (folio template-authoring-system/specification.md,
  617L) — different spec structure from portal/tick for diversity.

Regression: 119/119 tests green. Bundle 132.9 KB (up 1.3 KB,
17 KB under the 150 KB ceiling).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
leeovery added a commit that referenced this pull request Apr 20, 2026
Items in metadata.pending that fail every catch-up retry stayed in the
queue forever — a permanent failure (renamed work unit, malformed file)
would cost 3 OpenAI calls per item per bulk-index run, indefinitely.

Each entry now carries an attempts counter. processPendingQueue checks
the counter at entry and evicts with a stderr warning once attempts
reach PENDING_MAX_ATTEMPTS (10). The catch block that previously
swallowed the retry failure now writes the failure back to the queue,
bumping the counter so eviction eventually fires.

status output updated to show attempt/max so users can see an item on
the verge of eviction.

Closes deferred-issues #2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants