Add plan references with keyed identifiers#91
Conversation
Introduce a Reference model that connects plans to external resources (repos, PRs, docs) and to other plans. Key features: - References have an optional key for stable semantic identity (e.g. auth-repo, impl-pr) that agents and humans can use to identify what a reference represents - Auto-extraction from plan content: markdown links, bare URLs, and reference-style link definitions ([key]: url "title") are parsed on every version save - URL classification: GitHub repos, PRs, Google Docs, Notion, Confluence, other plans, and generic links - Cross-plan linking via target_plan_id for plan-type references - Full REST API: CRUD on /api/v1/plans/:id/references plus a search endpoint to find plans by referenced URL - Tabbed plan UI showing Content and References tabs - Agent instructions updated to document the references API and encourage keyed references Amp-Thread-ID: https://ampcode.com/threads/T-019d9222-fc6d-72ca-a6ec-c538f8ace722 Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e648006f8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| end | ||
|
|
||
| # Match bare URLs that aren't already inside markdown link syntax | ||
| stripped = content.gsub(/\[([^\]]*)\]\(([^)]+)\)/, "").gsub(/^\[([^\]]+)\]:\s+\S+.*$/m, "") |
There was a problem hiding this comment.
Stop removing trailing content when stripping ref defs
The bare-URL extraction path currently does gsub(/...$/m, ""), and with /m the .*$ part spans newlines, so once a [key]: url definition appears it can delete everything to the end of the document. Any plain https://... links after that point are never scanned, which silently drops extracted references for valid content that mixes reference-style links and bare URLs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — removed the /m flag from the ref-def stripping regex so $ matches end-of-line instead of end-of-string. Updated by Amp 🤖
| user: current_user, | ||
| plan_type_id: plan_type&.id |
There was a problem hiding this comment.
Wrap plan creation and reference saves atomically
The plan is created before reference validation/saves run, but failures in later ref.save! calls are rescued and returned as 422. That means a request can report failure while still leaving a newly-created plan/version persisted, which is especially problematic for clients that retry after 422 and end up creating duplicates. The create path should be transactional across plan + references (or validate references first) to keep API semantics consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — wrapped plan creation + reference saves in ActiveRecord::Base.transaction so a ref validation failure rolls back the plan. Updated by Amp 🤖
| @plan.references.find_or_create_by!(url: url) do |r| | ||
| r.key = params[:reference][:key].presence | ||
| r.title = params[:reference][:title].presence | ||
| r.reference_type = ref_type | ||
| r.source = "explicit" |
There was a problem hiding this comment.
Update existing URL references instead of no-oping
Using find_or_create_by! with a block means the attributes in the block are applied only on insert. If the URL already exists (for example from auto-extraction), submitting the add-reference form does not promote it to explicit or apply the provided key/title, yet the UI still reports “Reference added.” This breaks the keyed-reference workflow for links already present in content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — switched from find_or_create_by! to find_or_initialize_by + assign_attributes so existing extracted references get promoted to explicit with the provided key/title. Updated by Amp 🤖
…te no-op - Remove /m flag from ref-def stripping regex so bare URLs after reference-style definitions are still scanned - Wrap plan create + reference saves in a transaction so a ref validation failure rolls back the plan - Switch web controller from find_or_create_by! to find_or_initialize_by + assign_attributes so submitting a URL that already exists (e.g. from auto-extraction) promotes it to explicit and applies the provided key/title Amp-Thread-ID: https://ampcode.com/threads/T-019d9222-fc6d-72ca-a6ec-c538f8ace722 Co-authored-by: Amp <amp@ampcode.com>
- References create/destroy now respond with turbo_stream to replace the references panel in-place instead of full-page redirect - Tab IDs prefixed with 'tab-' (tab-content, tab-references) to avoid collisions with heading anchors in plan content - Tabs controller pushes ?tab= query param to URL via replaceState so tab selection persists across navigation/reload - HTML fallback redirects include tab: 'references' param - Add Reference form restyled: inputs use .form-group convention, horizontal layout, moved above reference list - Delete button uses data-turbo-stream for inline updates Amp-Thread-ID: https://ampcode.com/threads/T-019d9222-fc6d-72ca-a6ec-c538f8ace722 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d9222-fc6d-72ca-a6ec-c538f8ace722 Co-authored-by: Amp <amp@ampcode.com>
…tests
- Fix NoMethodError: use helpers.content_tag in controller
- Broadcast references panel + count via ActionCable when a new
PlanVersion extracts references, so API edits update all viewers
- Add 19 system tests covering:
- Tab navigation (show/hide, URL persistence, switching back)
- Adding references inline via Turbo Stream (single, multiple,
auto-classification)
- Removing references inline with count update
- Auto-extraction from content edits (new versions, link removal,
explicit reference preservation, keyed reference-style links)
- Display (key badges, source labels, target=_blank, truncation)
- Fix stale association cache in extract_from_content spec
Amp-Thread-ID: https://ampcode.com/threads/T-019d9222-fc6d-72ca-a6ec-c538f8ace722
Co-authored-by: Amp <amp@ampcode.com>
Remove 14 filler tests that only checked server-rendered HTML (covered by model, service, and request specs). Keep 5 focused tests that verify real browser integration: - Stimulus tab controller: replaceState URL updates, CSS class toggling - Turbo Stream add: DOM replacement without page navigation - Sequential adds: <details> re-expansion after partial replacement - Turbo Stream delete: data-turbo-confirm dialog + DOM removal - Cross-tab count: badge persistence across Stimulus tab switches Amp-Thread-ID: https://ampcode.com/threads/T-019d9c4b-b022-74c4-a0a6-929c26e7ff63 Co-authored-by: Amp <amp@ampcode.com>
What
Adds a Reference model that connects plans to external resources (GitHub repos, PRs, design docs, other plans) with optional keyed identifiers for stable semantic meaning.
Why
Plans frequently mention external resources but there's no structured way to track those connections. References make plans discoverable by the resources they touch ("which plans reference this repo?") and keyed identifiers let agents attach semantic context ("this is the
auth-repo, this is theimpl-pr") that persists across edits.Key features
keyfield — optional short identifier (e.g.auth-repo,impl-pr) unique per plan, lowercase alphanumeric with hyphens/underscores[key]: url "title") are parsed from plan content on every version savetarget_plan_idfor bidirectional discovery/api/v1/plans/:id/referencesplus asearchendpoint to find plans by referenced URLChanges
Referencemodel, migration, factoryExtractFromContentservice (runs onPlanVersion#after_create_commit)