Skip to content

Commit c43185c

Browse files
macmadeclaude
andcommitted
feat: assemble FITS sections into blocks with dirty-tracking
Realize the "preserve untouched, re-render touched" model: a section serializes to its retained blocks byte-for-byte when unmodified, and re-renders from its model when it needs serialization. Because rendering can fail (an invalid keyword, a non-finite float, an over-long record), the byte accessor is made throwing rather than risking stale bytes after a future mutation: - Add FITSSection.serializedData(options:) throws, routing to the retained blocks when clean and to the model renderer when dirty, plus appendSerializedData(to:options:) as the aggregation primitive. - Turn `data` into a throwing convenience (get throws) on FITSSection and FITSFile, equal to serializing with the strict options. - Add a per-section needsSerialization flag (independent of the isFinalized lock) and a markNeedsSerialization() hook. - Render a header to its cards plus the END marker, space-padded to a whole 2880-byte block, and a data payload zero-padded to the block boundary, per FITS 4.0. An unmodified parsed file still round-trips byte-for-byte. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 18cf4c2 commit c43185c

5 files changed

Lines changed: 271 additions & 42 deletions

File tree

Docs/agent-plans/active/2026-07-11-write-support/write-support-plan.html

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,8 @@ <h4><span class="m-num">M3</span> Render <code>FITSProperty</code> to an 80-byte
348348

349349
<!-- M4 -->
350350
<div class="milestone">
351-
<h4><span class="m-num">M4</span> Assemble a <code>FITSSection</code> into blocks with dirty-tracking <span class="capsule notstarted">Not started</span></h4>
351+
<h4><span class="m-num">M4</span> Assemble a <code>FITSSection</code> into blocks with dirty-tracking <span class="capsule completed">Completed</span></h4>
352+
<p class="meta"><strong>Completed:</strong> 2026-07-11</p>
352353
<div class="field goal">
353354
<div class="label">Goal</div>
354355
<div class="body">Render a whole section to 2880-byte blocks from its model, while keeping clean sections byte-for-byte identical to their retained bytes. This is where "preserve untouched, re-render touched" is realized.</div>
@@ -363,8 +364,26 @@ <h4><span class="m-num">M4</span> Assemble a <code>FITSSection</code> into block
363364
<li>Tests in <code>SwiftFITSTests/FITSSection.swift</code>: a clean parsed section re-serializes byte-identically; a synthetically built/dirtied section renders correct blocks and re-parses equal.</li>
364365
</ul>
365366
</div>
366-
<div class="field notes-field"><div class="label">Implementation notes</div><div class="body placeholder"></div></div>
367-
<div class="field auto-field"><div class="label">Automated review</div><div class="body placeholder"></div></div>
367+
<div class="field notes-field"><div class="label">Implementation notes</div><div class="body">
368+
<p>Implemented with TDD (red → green). No new files (changes to <code>FITSSection</code>, <code>FITSFile</code>, and their tests).</p>
369+
<p><strong>Key design decision — throwing byte accessor (discussed with the user before coding).</strong> The plan says "route <code>data</code>/<code>appendData(to:)</code>", but rendering can throw (invalid keyword, non-finite float, over-long record from M2/M3), and a lazy dirty flag means the current bytes are only produced at serialize time. Keeping a non-throwing <code>data</code> would return <em>stale</em> bytes after a mutation. The user chose to make the byte accessor throwing so it is never stale:</p>
370+
<ul>
371+
<li><code>FITSSection.serializedData(options:) throws -&gt; Data</code> — the router: clean → retained blocks (byte-for-byte), dirty → render from the model. Plus <code>appendSerializedData(to:options:) throws</code> as the aggregation primitive.</li>
372+
<li><code>var data: Data { get throws }</code> on both <code>FITSSection</code> and <code>FITSFile</code> — a convenience equal to <code>serializedData(options: .strict)</code>. <code>FITSFile.data</code> changed from non-throwing to <code>get throws</code>; existing reads gained <code>try</code> (incl. <code>allTestFilesRoundTrip</code>). <em>(This pulls the section-level <code>serializedData</code> forward from M5; M5 still adds the file-level <code>serializedData(options:)</code> + <code>write</code> + validation.)</em></li>
373+
</ul>
374+
<p><strong>Dirty-tracking:</strong> a private <code>needsSerialization</code> flag (default <code>false</code>), independent of <code>isFinalized</code> (a section can be finalized <em>and</em> dirty). <code>markNeedsSerialization()</code> is the internal hook M7's mutations will call. Parsing never sets it, so clean sections always re-emit retained bytes — <code>allTestFilesRoundTrip</code> stays byte-identical. (<code>finalize</code> now reads a new non-throwing <code>retainedBytes</code> instead of <code>data</code>, equivalent for a clean section.)</p>
375+
<p><strong>Rendering</strong> (grounded in FITS 4.0 §3.3.1/§3.3.2): a header/extension renders its properties to cards (M3) + the <code>END</code> card, space-padded to a whole 2880-byte block; a data section zero-pads its payload to the boundary. A minimal internal <code>init(dataPayload:)</code> builds a synthetic dirty data section (M7/M8 foundation, and enables the data-render test).</p>
376+
<p><strong>Verification:</strong> <code>xcodebuild test</code><em>TEST SUCCEEDED</em>; <code>swift test</code> → 183 tests passed (+3). Regression <code>allTestFilesRoundTrip</code> holds (clean path byte-identical).</p>
377+
<p><strong>Automated review loop:</strong> 1 round; converged with no Critical/High/Medium findings.</p>
378+
</div></div>
379+
<div class="field auto-field"><div class="label">Automated review</div><div class="body">
380+
<p>Round 1 — no Critical/High/Medium. The reviewer traced the clean-path byte-identity (a clean section short-circuits before the renderer even under <code>.strict</code>), the finalize equivalence, the header/data render correctness (END card, ASCII guard, space vs zero padding), the <code>payload</code>/inits, and the <code>try</code> ripple (no <code>try</code> on a non-throwing expression; assertions unchanged). Findings:</p>
381+
<ul>
382+
<li><strong>Low — two alignment nits</strong> from the change: a <code>let</code> group in the new data-render test, and a previously-aligned <code>#expect</code> group broken by the <code>try</code> ripple (one sibling can't take <code>try</code>). <em>Fixed both</em> (re-aligned; the three <code>#expect</code> lines share a column since <code>try file.data.count</code> matches <code>file.sections.count</code> in width).</li>
383+
<li><strong>Info — empty payload renders to 0 bytes</strong> (no block). Internally consistent and only reachable via the internal init; flagged so it's a conscious decision when M7/M8 build real data segments (a real image data section should never be empty). <em>Deferred.</em></li>
384+
<li><strong>Info — <code>dataSize</code> under-estimates the <code>Data(capacity:)</code> hint for a dirty section</strong> (it sums retained blocks). Harmless — it is only a reservation hint and <code>dataSize</code> is already documented as the retained size. <em>Deferred.</em></li>
385+
</ul>
386+
</div></div>
368387
<div class="field review-field"><div class="label">Review comments</div><div class="body placeholder"></div></div>
369388
</div>
370389

SwiftFITS/FITSFile.swift

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -429,18 +429,28 @@ public class FITSFile: CustomStringConvertible
429429
try validate?( properties[ index ].value )
430430
}
431431

432-
/// The complete file contents, reconstructed by concatenating every section.
432+
/// The complete file contents, serialized with the ``strict`` options.
433+
///
434+
/// A convenience for serializing every section with
435+
/// ``FITSSerializationOptions/strict``. An unmodified parsed file yields its
436+
/// original bytes byte-for-byte; a file whose sections were modified
437+
/// re-renders those sections from their model, which can fail.
438+
///
439+
/// - Throws: Any ``FITSError`` raised while rendering a modified section.
433440
public var data: Data
434441
{
435-
let size = self.sections.reduce( 0 ) { $0 + $1.dataSize }
436-
var data = Data( capacity: size )
437-
438-
self.sections.forEach
442+
get throws
439443
{
440-
$0.appendData( to: &data )
441-
}
444+
let size = self.sections.reduce( 0 ) { $0 + $1.dataSize }
445+
var data = Data( capacity: size )
442446

443-
return data
447+
try self.sections.forEach
448+
{
449+
try $0.appendSerializedData( to: &data, options: .strict )
450+
}
451+
452+
return data
453+
}
444454
}
445455

446456
/// The primary header section, or `nil` if the file has no sections.

SwiftFITS/FITSSection.swift

Lines changed: 155 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ public class FITSSection: CustomStringConvertible
7171
/// disagree).
7272
private var isFinalized = false
7373

74+
/// Whether the section's model has diverged from its retained ``blocks`` and
75+
/// must be re-rendered on serialization instead of re-emitting those bytes.
76+
///
77+
/// A freshly-parsed section is clean (`false`); building one from a model, or
78+
/// (later) mutating one, sets this. It is independent of ``isFinalized``: a
79+
/// section can be both finalized (locked against block appends) and in need
80+
/// of re-serialization.
81+
private var needsSerialization = false
82+
83+
/// The data payload of a synthetically-built data section, or `nil` for a
84+
/// parsed section, whose bytes live in ``blocks``.
85+
private let payload: Data?
86+
7487
/// The parsed header records. Empty for data sections, and until
7588
/// ``finalize(options:)`` has run.
7689
///
@@ -122,44 +135,174 @@ public class FITSSection: CustomStringConvertible
122135
/// is not valid for the section kind.
123136
public init( kind: Kind, block: FITSBlock? ) throws
124137
{
125-
self.kind = kind
138+
self.kind = kind
139+
self.payload = nil
126140

127141
if let block = block
128142
{
129143
try self.append( block: block )
130144
}
131145
}
132146

133-
/// The total size, in bytes, of all blocks in the section.
147+
/// Creates a synthetic data section from a raw payload.
148+
///
149+
/// The section is marked as needing serialization, so
150+
/// ``serializedData(options:)`` renders the payload (zero-padded to the block
151+
/// boundary) rather than re-emitting retained blocks, of which it has none.
152+
///
153+
/// - Parameter dataPayload: The data-segment bytes, of any length.
154+
internal init( dataPayload: Data )
155+
{
156+
self.kind = .data
157+
self.payload = dataPayload
158+
self.needsSerialization = true
159+
self.isFinalized = true
160+
}
161+
162+
/// Marks the section as needing re-serialization from its model.
163+
///
164+
/// Mutating a section's properties or data must call this so
165+
/// ``serializedData(options:)`` re-renders from the model instead of
166+
/// re-emitting the retained blocks.
167+
internal func markNeedsSerialization()
168+
{
169+
self.needsSerialization = true
170+
}
171+
172+
/// The total size, in bytes, of the section's retained blocks.
173+
///
174+
/// This reflects the bytes as parsed; a section pending re-serialization may
175+
/// render to a different size.
134176
public var dataSize: Int
135177
{
136178
self.blocks.reduce( 0 ) { $0 + $1.data.count }
137179
}
138180

139-
/// The concatenated raw bytes of every block in the section.
181+
/// The section's retained raw bytes, exactly as parsed.
182+
private var retainedBytes: Data
183+
{
184+
var data = Data( capacity: self.dataSize )
185+
186+
self.blocks.forEach { data.append( $0.data ) }
187+
188+
return data
189+
}
190+
191+
/// The section's serialized bytes, rendered with the ``strict`` options.
192+
///
193+
/// A convenience for ``serializedData(options:)`` with
194+
/// ``FITSSerializationOptions/strict``. A clean parsed section yields its
195+
/// retained bytes byte-for-byte; a section pending re-serialization is
196+
/// rendered from its model, which can fail.
197+
///
198+
/// - Throws: Any ``FITSError`` raised while rendering a section that needs
199+
/// serialization.
140200
public var data: Data
201+
{
202+
get throws
203+
{
204+
try self.serializedData( options: .strict )
205+
}
206+
}
207+
208+
/// The section's serialized bytes.
209+
///
210+
/// Returns the retained blocks unchanged when the section is clean (so an
211+
/// unmodified parsed section round-trips byte-for-byte), and renders from the
212+
/// model when the section needs serialization.
213+
///
214+
/// - Parameter options: The serialization options to apply when rendering.
215+
/// - Returns: The section's bytes, a whole number of 2880-byte blocks.
216+
/// - Throws: Any ``FITSError`` raised while rendering.
217+
public func serializedData( options: FITSSerializationOptions ) throws -> Data
141218
{
142219
var data = Data( capacity: self.dataSize )
143220

144-
self.appendData( to: &data )
221+
try self.appendSerializedData( to: &data, options: options )
145222

146223
return data
147224
}
148225

149-
/// Appends the section's block bytes, in order, to an existing buffer.
226+
/// Appends the section's serialized bytes to an existing buffer.
150227
///
151-
/// Lets a caller assemble several sections into one buffer without building
152-
/// an intermediate ``Data`` copy per section.
228+
/// Lets a caller assemble several sections into one buffer without an
229+
/// intermediate ``Data`` copy per section. Routes to the retained blocks when
230+
/// clean and to the model renderer when the section needs serialization.
153231
///
154-
/// - Parameter data: The buffer to append the section's block bytes to.
155-
internal func appendData( to data: inout Data )
232+
/// - Parameters:
233+
/// - data: The buffer to append to.
234+
/// - options: The serialization options to apply when rendering.
235+
/// - Throws: Any ``FITSError`` raised while rendering.
236+
internal func appendSerializedData( to data: inout Data, options: FITSSerializationOptions ) throws
156237
{
157-
self.blocks.forEach
238+
guard self.needsSerialization
239+
else
240+
{
241+
data.append( self.retainedBytes )
242+
243+
return
244+
}
245+
246+
switch self.kind
158247
{
159-
data.append( $0.data )
248+
case .header, .xtension: data.append( try self.renderedHeader( options: options ) )
249+
case .data: data.append( self.renderedDataSegment() )
160250
}
161251
}
162252

253+
/// Renders a header or extension section from its ``properties``.
254+
///
255+
/// Serializes every property to its card(s), appends the `END` marker, and
256+
/// blank-pads the result to a whole number of 2880-byte blocks (FITS 4.0
257+
/// §3.3.1).
258+
///
259+
/// - Parameter options: The serialization options to apply.
260+
/// - Returns: The rendered header bytes.
261+
/// - Throws: ``FITSError/cannotSerialize(reason:)`` if the rendered text is
262+
/// not ASCII, or any error raised while rendering a property.
263+
private func renderedHeader( options: FITSSerializationOptions ) throws -> Data
264+
{
265+
let cards = try self.properties.flatMap { try $0.serialized( options: options ) }
266+
let end = "END".padding( toLength: FITSFile.cardSize, withPad: " ", startingAt: 0 )
267+
268+
guard let ascii = ( cards + [ end ] ).joined().data( using: .ascii )
269+
else
270+
{
271+
throw FITSError.cannotSerialize( reason: "Header contains non-ASCII characters" )
272+
}
273+
274+
return FITSSection.paddedToBlockBoundary( ascii, fill: 0x20 )
275+
}
276+
277+
/// Renders a data section from its payload, zero-padded to the block boundary
278+
/// (FITS 4.0 §3.3.2).
279+
///
280+
/// - Returns: The rendered data-segment bytes.
281+
private func renderedDataSegment() -> Data
282+
{
283+
FITSSection.paddedToBlockBoundary( self.payload ?? self.retainedBytes, fill: 0x00 )
284+
}
285+
286+
/// Pads a buffer to a whole number of 2880-byte blocks.
287+
///
288+
/// - Parameters:
289+
/// - data: The bytes to pad.
290+
/// - fill: The byte to pad with (ASCII space for headers, zero for data).
291+
/// - Returns: `data` padded to the next block boundary, or unchanged if it is
292+
/// already block-aligned.
293+
private static func paddedToBlockBoundary( _ data: Data, fill: UInt8 ) -> Data
294+
{
295+
let remainder = data.count % FITSFile.blockSize
296+
297+
guard remainder != 0
298+
else
299+
{
300+
return data
301+
}
302+
303+
return data + Data( repeating: fill, count: FITSFile.blockSize - remainder )
304+
}
305+
163306
/// Appends a block to the section.
164307
///
165308
/// For header and extension sections this enforces structural rules: the
@@ -234,7 +377,7 @@ public class FITSSection: CustomStringConvertible
234377

235378
if self.kind == .header || self.kind == .xtension
236379
{
237-
let data = self.data
380+
let data = self.retainedBytes
238381

239382
if options.contains( .allowNonPrintableHeaderText ) == false, data.containsOnlyFITSPrintable == false
240383
{

SwiftFITSTests/FITSFile.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ struct Test_FITSFile
5757
let data = try Data( contentsOf: url )
5858
let file = try FITSFile( data: data, options: .lenient )
5959

60-
#expect( file.data == data, "Round-trip mismatch for \( url.lastPathComponent )" )
60+
#expect( try file.data == data, "Round-trip mismatch for \( url.lastPathComponent )" )
6161
}
6262
}
6363

@@ -185,7 +185,7 @@ struct Test_FITSFile
185185
let file = try FITSFile( url: url, options: .lenient )
186186
let copy = try FITSFile( data: file.data, options: .lenient )
187187

188-
#expect( file.data == copy.data )
188+
#expect( try file.data == copy.data )
189189
#expect( file.description == copy.description )
190190
}
191191

@@ -479,7 +479,7 @@ struct Test_FITSFile
479479
let file = try FITSFile( data: block, options: .lenient )
480480

481481
#expect( file.header?.properties.contains { $0.name == "CONTINUE" } == true )
482-
#expect( file.data == block )
482+
#expect( try file.data == block )
483483
}
484484

485485
@Test
@@ -535,7 +535,7 @@ struct Test_FITSFile
535535
try #require( file.sections.count == 2 )
536536

537537
#expect( file.sections[ 1 ].kind == .data )
538-
#expect( file.data == header + data )
538+
#expect( try file.data == header + data )
539539
}
540540

541541
@Test
@@ -551,7 +551,7 @@ struct Test_FITSFile
551551
#expect( file.sections.count == 1 )
552552
#expect( file.sections.allSatisfy { $0.kind != .data } )
553553
#expect( file.extensions.isEmpty )
554-
#expect( file.data == header + padding )
554+
#expect( try file.data == header + padding )
555555
}
556556

557557
@Test
@@ -592,7 +592,7 @@ struct Test_FITSFile
592592
#expect( file.sections[ 3 ].kind == .data )
593593
#expect( file.extensions.count == 1 )
594594

595-
#expect( file.data == header + data1 + ext + data2 )
595+
#expect( try file.data == header + data1 + ext + data2 )
596596
}
597597

598598
@Test
@@ -663,8 +663,8 @@ struct Test_FITSFile
663663
let file = try FITSFile( data: data, options: .lenient )
664664

665665
#expect( file.sections.count == 1 )
666-
#expect( file.data.count == FITSFile.blockSize * 2 )
667-
#expect( file.data == header + partial + Data( repeating: 0x00, count: FITSFile.blockSize - 100 ) )
666+
#expect( try file.data.count == FITSFile.blockSize * 2 )
667+
#expect( try file.data == header + partial + Data( repeating: 0x00, count: FITSFile.blockSize - 100 ) )
668668
}
669669

670670
@Test
@@ -695,7 +695,7 @@ struct Test_FITSFile
695695
#expect( file.sections.count == 1 )
696696
#expect( file.header?.properties.last?.name == "FOO" )
697697
#expect( file.header?.properties.last?.value.integer == 1 )
698-
#expect( file.data == block )
698+
#expect( try file.data == block )
699699
}
700700

701701
@Test

0 commit comments

Comments
 (0)