fix: apply suggestion content to page body on accept

[khasinski]

Summary

• Implement apply_suggestion in Suggestable concern for edit/delete/add types (was a placeholder that only updated status)
• Page overrides suggestable_content to read/write markdown body
• Finding overrides suggestable_content to read/write description
• Controller returns error response when content mutation fails

Previously accepting a suggestion only flipped its status to "accepted" -- the actual page content was never modified, so changes were lost on reload.

Test plan

• Full suite: 293 runs, 744 assertions, 0 failures, 0 errors

[superconductor-for-github]

✅ @khasinski — Superconductor finished — View implementation

---

Standing by for instructions.

[Copilot]

Pull request overview

This pull request implements the core functionality for applying suggestions to page and finding content. Previously, accepting a suggestion only updated its status to "accepted" without actually modifying the underlying content, causing changes to be lost on page reload. The PR adds concrete implementations of the apply_suggestion workflow that was only a placeholder before.

Changes:

• Implemented apply_suggestion method in Suggestable concern with support for edit, delete, and add suggestion types
• Added suggestable_content and update_suggestable_content methods to Page model for accessing/updating markdown body content
• Added suggestable_content and update_suggestable_content methods to Finding model for accessing/updating description field

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
app/models/concerns/suggestable.rb	Implements the core logic for applying edit/delete/add suggestions by reading content, modifying it based on suggestion type, and updating the record
app/models/page.rb	Provides access to markdown body content for suggestion application via ActionText integration
app/models/finding.rb	Provides access to description field for suggestion application
app/controllers/suggestions_controller.rb	Updates accept action to call apply_suggestion and handle errors with appropriate responses

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The apply methods read content, modify it, and save it in separate operations, which creates a potential race condition. If two suggestions are applied concurrently to the same suggestable record, both could read the same initial content, and one update could overwrite the other. Consider using optimistic locking (lock_version) or wrapping the read-modify-write operations in a transaction with appropriate locking to prevent concurrent modifications.

Suggested change

	content = suggestable_content
	return false unless content&.include?(suggestion.original_text)
	update_suggestable_content(content.sub(suggestion.original_text, suggestion.suggested_text))
	end
	def apply_delete(suggestion)
	return false if suggestion.original_text.blank?
	content = suggestable_content
	return false unless content&.include?(suggestion.original_text)
	update_suggestable_content(content.sub(suggestion.original_text, ""))
	end
	def apply_add(suggestion)
	return false if suggestion.suggested_text.blank?
	content = suggestable_content || ""
	update_suggestable_content(content + "\n\n" + suggestion.suggested_text)
	with_lock do
	content = suggestable_content
	next false unless content&.include?(suggestion.original_text)
	update_suggestable_content(content.sub(suggestion.original_text, suggestion.suggested_text))
	end
	end
	def apply_delete(suggestion)
	return false if suggestion.original_text.blank?
	with_lock do
	content = suggestable_content
	next false unless content&.include?(suggestion.original_text)
	update_suggestable_content(content.sub(suggestion.original_text, ""))
	end
	end
	def apply_add(suggestion)
	return false if suggestion.suggested_text.blank?
	with_lock do
	content = suggestable_content || ""
	update_suggestable_content(content + "\n\n" + suggestion.suggested_text)
	end

[Copilot]

The content update and suggestion status update are not wrapped in a transaction. If the content update succeeds but suggestion.accept! fails (e.g., due to a database error), the content will have been modified but the suggestion will remain in "pending" status. This could allow the same suggestion to be applied multiple times. Consider wrapping both operations in a transaction to ensure atomicity.

Suggested change

	case suggestion.suggestion_type
	when "edit"
	return false unless apply_edit(suggestion)
	when "delete"
	return false unless apply_delete(suggestion)
	when "add"
	return false unless apply_add(suggestion)
	end
	suggestion.accept!
	true
	success = false
	ActiveRecord::Base.transaction do
	case suggestion.suggestion_type
	when "edit"
	raise ActiveRecord::Rollback unless apply_edit(suggestion)
	when "delete"
	raise ActiveRecord::Rollback unless apply_delete(suggestion)
	when "add"
	raise ActiveRecord::Rollback unless apply_add(suggestion)
	end
	suggestion.accept!
	success = true
	end
	success

[Copilot]

The methods apply_edit, apply_delete, and apply_add don't return explicit values on success. The update_suggestable_content method is called but its return value is used as the return value of these methods. However, the default implementation of update_suggestable_content returns false, and the overridden implementations use save! or update! which return true on success. For consistency and clarity, consider explicitly returning true after successful content updates, or ensure all implementations document their return values.

[Copilot]

The save! method can raise ActiveRecord exceptions (e.g., validation errors, database errors) that are not rescued here. If saving fails, the exception will propagate up to the controller. While the controller handles the boolean return value from apply_suggestion, it doesn't catch exceptions. Consider either rescuing exceptions here and returning false, or documenting that this method may raise exceptions.

[Copilot]

The update! method can raise ActiveRecord exceptions (e.g., validation errors, database errors) that are not rescued here. If the update fails, the exception will propagate up to the controller. While the controller handles the boolean return value from apply_suggestion, it doesn't catch exceptions. Consider either rescuing exceptions here and returning false, or documenting that this method may raise exceptions.

[Copilot]

The case statement doesn't explicitly handle the "comment" suggestion type, which is a valid type according to the Suggestion model. Currently, comment-type suggestions will fall through the case statement and be accepted without any content changes, which may be the intended behavior. However, this is implicit and could be confusing. Consider adding an explicit when "comment" branch (which could just be empty or contain a comment explaining that comments don't modify content) to make the intent clear.

Suggested change

	return false unless apply_add(suggestion)
	return false unless apply_add(suggestion)
	when "comment"
	# Comment suggestions do not modify content; accept as-is.
	# No-op