Improve ledger format #4

Closed
opened 2026-05-14 08:05:20 +00:00 by erik · 6 comments
Owner

Agents are duplicating status details in their ledgers. Let simplify for them. Instead of giving them the full comment text, let's split a ledger in two:

fjx-managed
---
agent-managed

That first --- is the separator (similar to how yaml frontmatter is handled). fjx should always only show the agent-managed text. Agent prompts should be updated (hopefully simplified) so that they do not worry about managing that metadata. Instead, the ledger commands will require a status arg. fjx will use that status and current time for checkpoint. To review, all ledgers have this metadata item in common:

Status: <status> @ <checkpoint>

The PM is currently only one with extra Briefs metadata.

Agents are duplicating status details in their ledgers. Let simplify for them. Instead of giving them the full comment text, let's split a ledger in two: ``` fjx-managed --- agent-managed ``` That first `---` is the separator (similar to how yaml frontmatter is handled). fjx should always only show the agent-managed text. Agent prompts should be updated (hopefully simplified) so that they do not worry about managing that metadata. Instead, the ledger commands will require a `status` arg. fjx will use that status and current time for checkpoint. To review, all ledgers have this metadata item in common: ``` Status: <status> @ <checkpoint> ``` The PM is currently only one with extra Briefs metadata.
Member

Brief — Developer

Phase: simple. Read wiki/workflow/prompts/dev-simple.md for the cycle protocol.

<!-- pm:brief:dev --> # Brief — Developer **Phase:** simple. Read `wiki/workflow/prompts/dev-simple.md` for the cycle protocol.
Member

Ledger for: agent-pm

Status: watchingclosed @ 2026-05-14T08:37:06.922Z
Briefs:

  • dev @ 2026-05-14T08:06:05Z
  • test @ 2026-05-14T08:37:26Z
  • build: absent

Activity

_(none)_1. i4/closed : applied

Bugs Found

  • fjx pm tick /note parsing gap: /note commands in issue comments are not captured in activityEntries. This breaks the workflow where /note should route through pm close for notes_for_review[] and wiki promotion. The /note on this issue (id#984, "I over-rode @agent-test...") was present in Forgejo but not captured by the sweep at 09:16:19Z.
<!-- agent-pm:ledger --> ## Ledger for: agent-pm Status: watchingclosed @ 2026-05-14T08:37:06.922Z Briefs: - [dev](#issuecomment-841) @ 2026-05-14T08:06:05Z - [test](#issuecomment-942) @ 2026-05-14T08:37:26Z - build: absent ### Activity _(none)_1. i4/closed : applied ### Bugs Found - **fjx pm tick `/note` parsing gap**: `/note` commands in issue comments are not captured in `activityEntries`. This breaks the workflow where `/note` should route through `pm close` for `notes_for_review[]` and wiki promotion. The `/note` on this issue (id#984, "I over-rode @agent-test...") was present in Forgejo but not captured by the sweep at 09:16:19Z.
Member

Ledger for: agent-dev

Status: review @ 2026-05-14T08:31:37.344Z


Tasks

  • Implement ledger format split (fjx-managed / agent-managed) — commit 5dd35e6
  • Update ledger commands to require status arg — commit 5dd35e6
  • Update agent prompts to remove ledger metadata handling — commit 5dd35e6
  • Test and validate changes — all 85 tests pass, validation clean

Summary

Completed Issue #4: Improve Ledger Format. Split ledgers into fjx-managed (metadata) and agent-managed (content) sections separated by ---. Updated ledger commands to require --status arg; fjx handles checkpoint computation. Simplified agent prompts to not manage metadata. All tests pass.

Handoff: PR #8 created and marked agent/review.

<!-- agent-dev:ledger --> ## Ledger for: agent-dev Status: review @ 2026-05-14T08:31:37.344Z --- ### Tasks - [x] Implement ledger format split (fjx-managed / agent-managed) — commit 5dd35e6 - [x] Update ledger commands to require status arg — commit 5dd35e6 - [x] Update agent prompts to remove ledger metadata handling — commit 5dd35e6 - [x] Test and validate changes — all 85 tests pass, validation clean ### Summary Completed Issue #4: Improve Ledger Format. Split ledgers into fjx-managed (metadata) and agent-managed (content) sections separated by `---`. Updated ledger commands to require `--status` arg; fjx handles checkpoint computation. Simplified agent prompts to not manage metadata. All tests pass. **Handoff**: PR #8 created and marked agent/review.
Member

Brief — Tester

Phase: review. Read wiki/workflow/prompts/test.md for the cycle protocol.

<!-- pm:brief:test --> # Brief — Tester **Phase:** review. Read `wiki/workflow/prompts/test.md` for the cycle protocol.
Member

Ledger for: agent-test

Status: concerns @ 2026-05-14T08:49:48.385Z


Ledger for: agent-test

Status: concerns @ 2026-05-14T08:49:24.597Z

Action Summary

  • Workflow: validate.yaml
  • Verdict: success (all 85 unit tests pass; format/lint/types clean)
  • Coverage: 51.4% lines (maintained); commands/ledger.ts only 22.4% covered

Gap Analysis: Untested Ledger Format Changes

The PR introduces a critical feature — splitting ledgers into fjx-managed and agent-managed sections separated by --- — but the test coverage exposes significant gaps:

1. Missing Tests for extractAgentManagedSection() (NEW FUNCTION)

  • Function defined in src/util.ts but never called or tested
  • No test cases for:
    • Extracting section when separator exists: "fjx\n---\nagent""agent"
    • Backward compatibility when no separator (fallback to whole body)
    • Multiple --- in content (must only split on \n---\n)
    • Separator at various positions (start, middle, end)

2. Missing Tests for Ledger Format with Separator

  • Dev ledger tests (dev.test.ts) still use old format without separator
  • No test verifies:
    • Ledger write reconstructs format correctly: fjx-managed\n\n---\n\nagent-managed
    • Checkpoint in Status line has correct timestamp format
    • Agent-managed section is preserved exactly (no trimming artifacts)
    • Round-trip: write → read → extract → parse produces original content

3. Missing Test for --status Requirement

  • New --status flag is required when writing (line 55–58 ledger.ts)
  • No test for:
    • Error exit code 1 when --status is missing
    • Various status values ("working", "review", etc.) are accepted
    • Status appears correctly in reconstructed ledger

4. No Integration Test of ledger read path

  • dev.test.ts tests upsertCommentByMarker (marker layer) but NOT the ledger command layer with new format
  • Missing test that:
    • Creates ledger with fjx dev ledger --body-file X --status working
    • Reads it back with fjx dev ledger
    • Verifies output structure matches expectations

Confirmed Issues

None detected. Behavior observed in actions matches implementation; no runtime errors or assertion failures.

Untested Assumptions

  1. Backward compatibility: Ledgers without --- are handled gracefully by agents reading them. Not verified end-to-end.
  2. Separator robustness: Edge case where agent-managed section contains --- on its own line. Current code splits on \n---\n (literal string), so it should work, but never tested.
  3. Status validation: The code accepts any string for status (working, review, etc.). No enum or validation; assumes agents send valid values.

Verdict

concerns — Actions pass and code quality is solid, but the ledger format refactor introduces untested code paths critical to agent workflows. The extractAgentManagedSection() function exists but is never exercised, suggesting incomplete implementation or dead code. Gap-filling tests would catch issues agents will hit in production.

Recommend: Add unit tests for separator parsing and --status validation before merge. Add integration test of round-trip ledger write → read → extract cycle.

<!-- agent-test:ledger --> ## Ledger for: agent-test Status: concerns @ 2026-05-14T08:49:48.385Z --- ## Ledger for: agent-test Status: concerns @ 2026-05-14T08:49:24.597Z ### Action Summary - Workflow: validate.yaml - Verdict: success (all 85 unit tests pass; format/lint/types clean) - Coverage: 51.4% lines (maintained); commands/ledger.ts only 22.4% covered ### Gap Analysis: Untested Ledger Format Changes The PR introduces a critical feature — splitting ledgers into fjx-managed and agent-managed sections separated by `---` — but the test coverage exposes significant gaps: **1. Missing Tests for `extractAgentManagedSection()` (NEW FUNCTION)** - Function defined in src/util.ts but never called or tested - No test cases for: - Extracting section when separator exists: `"fjx\n---\nagent"` → `"agent"` - Backward compatibility when no separator (fallback to whole body) - Multiple `---` in content (must only split on `\n---\n`) - Separator at various positions (start, middle, end) **2. Missing Tests for Ledger Format with Separator** - Dev ledger tests (dev.test.ts) still use old format without separator - No test verifies: - Ledger write reconstructs format correctly: `fjx-managed\n\n---\n\nagent-managed` - Checkpoint in Status line has correct timestamp format - Agent-managed section is preserved exactly (no trimming artifacts) - Round-trip: write → read → extract → parse produces original content **3. Missing Test for `--status` Requirement** - New `--status` flag is required when writing (line 55–58 ledger.ts) - No test for: - Error exit code 1 when `--status` is missing - Various status values ("working", "review", etc.) are accepted - Status appears correctly in reconstructed ledger **4. No Integration Test of ledger read path** - dev.test.ts tests `upsertCommentByMarker` (marker layer) but NOT the ledger command layer with new format - Missing test that: - Creates ledger with `fjx dev ledger --body-file X --status working` - Reads it back with `fjx dev ledger` - Verifies output structure matches expectations ### Confirmed Issues **None detected.** Behavior observed in actions matches implementation; no runtime errors or assertion failures. ### Untested Assumptions 1. **Backward compatibility:** Ledgers without `---` are handled gracefully by agents reading them. Not verified end-to-end. 2. **Separator robustness:** Edge case where agent-managed section contains `---` on its own line. Current code splits on `\n---\n` (literal string), so it should work, but never tested. 3. **Status validation:** The code accepts any string for status (working, review, etc.). No enum or validation; assumes agents send valid values. ### Verdict **concerns** — Actions pass and code quality is solid, but the ledger format refactor introduces untested code paths critical to agent workflows. The `extractAgentManagedSection()` function exists but is never exercised, suggesting incomplete implementation or dead code. Gap-filling tests would catch issues agents will hit in production. Recommend: Add unit tests for separator parsing and --status validation before merge. Add integration test of round-trip ledger write → read → extract cycle.
erik closed this issue 2026-05-14 09:13:26 +00:00
Author
Owner

/note I over-rode @agent-test since extractAgentManagedSection() is so simple and an e2e round-trip test is not worth hassle at this point.

/note I over-rode @agent-test since extractAgentManagedSection() is so simple and an e2e round-trip test is not worth hassle at this point.
Sign in to join this conversation.
No description provided.