dev-docs: introduce application_model.md as entry point, normalize naming, clean work/#134
dev-docs: introduce application_model.md as entry point, normalize naming, clean work/#134
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces a central developer entry point ChangesDocumentation and navigation rework
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dev-docs/application_model.md (1)
361-361: Optional: Simplify redundant phrase.The phrase "in-flight or proposed plan" could be shortened to just "plan" since "in-flight" and "proposed" both already imply it's a plan.
✨ Suggested simplification
- → [`work/`](../work/) — each `.md` is an in-flight or proposed plan. + → [`work/`](../work/) — each `.md` is a plan (in-flight or proposed).or simply:
- → [`work/`](../work/) — each `.md` is an in-flight or proposed plan. + → [`work/`](../work/) — each `.md` documents planned work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-docs/application_model.md` at line 361, Replace the redundant phrase in the documentation entry "→ [`work/`](../work/) — each `.md` is an in-flight or proposed plan." by changing "in-flight or proposed plan" to simply "plan" so the line reads "→ [`work/`](../work/) — each `.md` is a plan."; update the corresponding text in dev-docs/application_model.md where that exact phrase appears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@dev-docs/application_model.md`:
- Line 361: Replace the redundant phrase in the documentation entry "→
[`work/`](../work/) — each `.md` is an in-flight or proposed plan." by changing
"in-flight or proposed plan" to simply "plan" so the line reads "→
[`work/`](../work/) — each `.md` is a plan."; update the corresponding text in
dev-docs/application_model.md where that exact phrase appears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb304ab2-92ee-4115-9c32-5cb7c1daf4ae
📒 Files selected for processing (15)
CLAUDE.mdCONTRIBUTING.mdclaude_code_log/converter.pydev-docs/agents.mddev-docs/application_model.mddev-docs/css-classes.mddev-docs/dag.mddev-docs/implementing-a-tool-renderer.mddev-docs/message-hierarchy.mddev-docs/messages.mddev-docs/rendering-architecture.mddev-docs/teammates.mddocs/restoring-archived-sessions.mdwork/phase-c-agent-transcripts.mdwork/rendering-next.md
💤 Files with no reviewable changes (1)
- work/phase-c-agent-transcripts.md
…lize naming
Split planning content from as-built reference content per the principle
that ``work/`` holds plans/TODOs (authoritative until the code lands)
and ``dev-docs/`` reflects the current code (authoritative once the
code lands).
Three file moves, one rename, one deletion, one new file:
- ``dev-docs/FOLD_STATE_DIAGRAM.md`` → ``dev-docs/message-hierarchy.md``
(drops the SCREAMING_SNAKE; H1 also updated to "Message Hierarchy
and Fold State" to match content scope).
- ``dev-docs/restoring-archived-sessions.md`` → ``docs/restoring-archived-sessions.md``
(user-facing operations content; ``docs/`` is the new home for
user-facing documentation distinct from contributor docs).
- ``dev-docs/rendering-next.md`` → ``work/rendering-next.md``
("Future Work for the rendering system" — explicitly plan content).
- ``work/phase-c-agent-transcripts.md`` deleted: the work fully landed
via PR #99 (b5aefa0); the as-built reality is now in ``agents.md``.
- ``dev-docs/application_model.md`` (new): entry-point doc.
Inbound references updated in ``CLAUDE.md``, ``CONTRIBUTING.md``,
``claude_code_log/converter.py`` (URL to docs/), and the four dev-docs
that cross-referenced the moved/renamed files.
The new ``application_model.md`` is the entry point: § 1 subsystem
table with pointers to deep-dive docs, § 2 inlined coverage of
subsystems without their own deep-dive (CLI, TUI, cache implementation,
migrations, detail-level filter, image export, performance profiling),
§ 3 data lifecycle ASCII diagram, § 4 cross-cutting glossary
(TranscriptEntry, MessageContent, TemplateMessage, RenderingContext,
session_id and synthetic forms, sidechain, agent_id, fork point,
pair_first/middle/last, detail level, passthrough), § 5 "where to
start reading" by question.
Surviving dev-docs all carry a one-line breadcrumb pointing at
``application_model.md`` so a reader landing on a deep-dive without
context can find the system overview.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-through A reviewer reading the new ``application_model.md`` cold landed at ``dag.md`` for branch sessions, but the doc was silent on where the branch-heading text itself is assembled — they'd have had to grep ``renderer.py`` to find it. Three small fixes close the gap: 1. ``SessionHeaderMessage`` glossary entry in ``application_model.md`` § 4: defines the term, distinguishes trunk vs branch flavours, names the four functions that compose the title (``_branch_label``, ``_enrich_branch_titles``, ``create_session_preview``, ``simplify_command_tags``). Lands between the ``fork point / branch`` and ``pair_first / middle / last`` entries since branch headings are the lookup most likely to lead here. 2. Cross-reference in ``dag.md``'s "Navigation Links" section: one-line callout that title composition is a renderer concern, pointing back at the new glossary entry. Keeps ``dag.md`` focused on structure (its job) without the reader hitting a silent dead end on the labeling question. 3. Cross-cutting-concerns paragraph after the § 1 subsystem table: names "label and preview composition" as the canonical example of a concern that touches several rows (DAG, renderer, parsing) and isn't owned by any single one. Helps the next reader who looks for "where do branch labels live?" and finds neither the DAG row nor the renderer row obviously authoritative. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…SR1), #133 (factory-layer normalisation seam) Three PRs landed on main since this branch was created. The dev-docs deltas they imply: - **PR #36 (JSON renderer)** — - `application_model.md` § 1 subsystem table gains a JSON export row; new § 2.5 "JSON export" describes the renderer (mirrors HTML/Markdown surface, runs through `generate_template_messages` so it inherits all post-factory polishing for free, JSON-specific `_json_default` shim for embedded Pydantic models, output naming via `variant_suffix`). - § 2.1 CLI lists `json` alongside `html`/`md`/`markdown` for `--format`. - § 3 data lifecycle diagram adds `json/renderer.py` as a third fan-out branch alongside HTML/Markdown. - § 5 entry-questions gains a "How do I export to JSON?" pointer. - `rendering-architecture.md` § 1 / § 4 list JSON; new `JsonRenderer` subsection in § 7 describes its serialisation approach (single document via `dataclasses.asdict` + shim; only reuses `title_content` from the dispatcher). - § 10 Separation of Concerns clarifies that **factories/** is the normalisation seam — display polish for *all* output formats lives there, not in renderers. - `implementing-a-tool-renderer.md` Overview + Tests sections note that JSON needs no per-tool integration: serialises the typed factory output directly. - **PR #135 (DAG cycle-break + SIGUSR1)** — - `dag.md` Phase 2 (Build DAG) gains a mermaid diagram of the three-step `build_dag` ordering (orphan promotion → cycle break → children build) and explains the ordering rationale (children must be built last because cyclic edges in `parent_uuid` would propagate into `children_uuids` otherwise, hanging downstream walks). - Phase 3 documents the `walk_visited` defence-in-depth in `_walk_session_with_forks` — backstop for reintroduced cycles after build time. - Invariant 3 (DAG acyclicity) updated from "no cycles" assertion to "actively broken at build time, walker has belt". - `application_model.md` new § 2.9 "Diagnosing hangs" describes the SIGUSR1 stack-dump handler installed in `cli.py`. - **PR #133 (system-info-cleanup)** — - Already mostly absorbed via the branch-heading navigation gap commit; the renormalisation ("if it shows up right in HTML, it shows up right in JSON") and the explicit "factory-layer normalisation seam" framing in `rendering-architecture.md` § 10 cite `simplify_command_tags` as the canonical example. Also drive-by: § 2.6 detail-level table now includes the `high` level (was missing — `models.DetailLevel.HIGH` predates this branch but the doc never listed it). `just ci` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cboos
force-pushed
the
dev/dev-docs-restructure
branch
from
027f8c6 to
efce0d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev-docs/application_model.md`:
- Around line 75-76: The doc text misstates the scope of the --page-size flag:
update the sentence that currently reads "`--page-size N` — split per-session
HTML into N-message pages." to clarify that pagination applies to the combined
transcript output (e.g., "split the combined transcript HTML into N-message
pages") so readers understand --page-size affects the merged/combined transcript
output rather than individual session files; keep the flag name `--page-size` in
the updated sentence for clarity.
- Around line 230-237: Update the image export mode names in the documentation
to match the runtime/CLI terminology: replace `inline` with `embedded`,
`dropped` with `placeholder`, and keep `referenced`; also update the default
sentence to state "embedded for Markdown (single self-contained file) and
referenced for HTML (small files, no inline payload bloat)". Verify the
surrounding text that enumerates modes (the list containing
`inline/referenced/dropped`) and any examples or flag references to use
`embedded/referenced/placeholder` so contributors use the correct CLI flag
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea6efe2d-e1f9-415f-96e7-832d2599f68b
📒 Files selected for processing (15)
CLAUDE.mdCONTRIBUTING.mdclaude_code_log/converter.pydev-docs/agents.mddev-docs/application_model.mddev-docs/css-classes.mddev-docs/dag.mddev-docs/implementing-a-tool-renderer.mddev-docs/message-hierarchy.mddev-docs/messages.mddev-docs/rendering-architecture.mddev-docs/teammates.mddocs/restoring-archived-sessions.mdwork/phase-c-agent-transcripts.mdwork/rendering-next.md
💤 Files with no reviewable changes (1)
- work/phase-c-agent-transcripts.md
✅ Files skipped from review due to trivial changes (1)
- work/rendering-next.md
…rt modes)
Two doc claims didn't match the actual code; both verified directly
against `cli.py` / `converter.py` / `image_export.py` rather than the
CR text alone.
- L75-76 page-size scope: clarify pagination targets the
combined-transcript output (whole-session packing into N-message
pages, with overflow when a single session exceeds N), not
per-session HTML files. Verified against `cli.py:547-551` (help
text says "combined transcript") and `converter.py:1002`
`_assign_sessions_to_pages` (per-page accumulation at the
combined-transcript level).
- § 2.7 image-export modes: rewrite to match the actual CLI choices.
Mode names were wrong (`inline/referenced/dropped` →
`placeholder/embedded/referenced`); defaults were also swapped
(now `embedded` for HTML, `referenced` for Markdown). Verified
against `cli.py:541-544` and `image_export.py:29-45`.
Skipped CR's L361 nit ("each `.md` is an in-flight or proposed plan"
→ "each `.md` is a plan"): the longer phrasing carries information
about work/'s mixed state and isn't redundant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This is great! The only thing I'm wondering is if we should add some strongly worded exhortation in CLAUDE.md for Claude to keep it updated when doing bigger changes? |
|
Some context: at the beginning, Claude Code was eager to throw in lots of .md documents, left and right. You may remember that I committed some by accident at random places in the project ;-) This aligns with my own practice: start with documents in work/ (I even have work/future/, work/archive/, but then I don't have GitHub PRs, so that would be overkill here). Sharing the branch's work/document is still useful, I think, and when the feature is implemented, the material in there can be integrated into dev-docs/, as such, or dispatched to existing docs, and, on this occasion, updated to match what was implemented. The document status evolves, from authoritative spec in work/ for guiding initial development, to WIP scratchpad (the spec evolves as the code adapts to reality), to dev-docs/ where it's no longer a spec but informational (the code is now the authoritative part). So, yes, there's a need to keep dev-docs/ "in sync" as the code evolves, if we want it to be useful and not misleading. I do that from time to time, and I noticed that Claude Code does it as well, but this could be made more systematic with appropriate instructions in CLAUDE.md. |
Per discussion on PR #134: dev-docs/ are useful only if they remain true to the as-built reality. Articulate the principle (code is authoritative; doc is wrong if they disagree) and the work/ → dev-docs/ lifecycle so non-trivial changes prompt a doc update in the same commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
(Claude) Good prompt — pushed
The first paragraph is the actionable directive (code is authoritative, fix the doc when they diverge). The second paragraph captures the Kept it terse per the project's CLAUDE.md style guide; left CONTRIBUTING.md untouched to avoid duplication. Open to wording tweaks if you want more or less force. |
2 participants
Summary
Restructures
dev-docs/andwork/so the directories cleanly separate as-built reference (dev-docs/) from plans/TODOs (work/), and introduces a lean entry-point document fordev-docs/.dev-docs/application_model.md— entry-point doc covering subsystems, data lifecycle, glossary, and inline coverage of subsystems without their own deep-dive (CLI, TUI, cache implementation, migrations, detail-level filter, image export, performance profiling).dev-docs/FOLD_STATE_DIAGRAM.md→dev-docs/message-hierarchy.md(drops the SCREAMING_SNAKE; H1 also updated).dev-docs/restoring-archived-sessions.md→docs/restoring-archived-sessions.md(user-facing operations;docs/is the new home for end-user documentation).dev-docs/rendering-next.md→work/rendering-next.md(it was always future-work content).work/phase-c-agent-transcripts.mddeleted: the work fully landed via Integrate agent transcripts into the DAG (Phase C) #99; the as-built reality is now inagents.md.CLAUDE.md,CONTRIBUTING.md,claude_code_log/converter.py(URL points atdocs/), and the fourdev-docs/*.mdfiles that cross-referenced the moved/renamed pages.application_model.mdso a reader landing on a deep-dive without context finds the system overview.Validation
A second commit (
027f8c6) folds in three follow-up fixes from a docs-walk-through with a fresh reader — they navigatedapplication_model.md→dag.md→messages.mdcleanly to investigate a hypothetical branch-heading rendering bug, but flagged that the label composition surface (_branch_label,_enrich_branch_titles,create_session_preview,simplify_command_tags) wasn't reachable from the docs alone:SessionHeaderMessageglossary entry naming the four functions involved.dag.md's "Navigation Links" section pointing at the glossary entry — keepsdag.mdfocused on structure without leaving a silent dead end on labeling.Resulting structure
Test plan
grep -rIn "FOLD_STATE_DIAGRAM\|phase-c-agent-transcripts"returns nothing outside test fixtures.dev-docs/*.mdresolve (relative paths verified).CLAUDE.mdandCONTRIBUTING.mdArchitecture sections updated to point atapplication_model.mdfirst.🤖 Generated with Claude Code
Summary by CodeRabbit