feat: NMC 3733 - Albums on mobile app customisation - #4091
feat: NMC 3733 - Albums on mobile app customisation#4091harshada-15-tsys wants to merge 2 commits into
Conversation
marinofaggiana
left a comment
There was a problem hiding this comment.
-> please use realm >= 20.0.0
-> The API must be moved to NextcloudKit 0.7.1
-> I need to rebase because I can't build it.
-> please use the Nextcloud standard version :-)
|
Hi @marinofaggiana , Understood! I'm updating to Realm 20.0.0+ and NextcloudKit 0.7.1 now, then I'll rebase to fix the build issues. |
|
@harshada-15-tsys yes, always use the master branch. |
|
Hi @harshada-15-tsys, if you need any help we can have a call. Please let us know and we can set up a room :) |
af6f039 to
826f10c
Compare
|
Hi @mpivchev, Thanks for the offer! I really appreciate it. I’ll keep that in mind and reach out if I need any help or want to discuss the updates further. I’ll share the revised code for Album with the suggested changes by next week. Apologies for the delayed response—I’ve been tied up with other priorities. |
|
Hi @harshada-15-tsys this is a cool feature, any chance you could find time to get it to the finishing line? Then it could go into the next major release and be a subject we can talk about ;-) |
|
@harshada-15-tsys can I help you finish the job if you don't have time to continue? |
|
@marinofaggiana Thanks for the understanding. |
…ed against Nothing here said how well the tool works, so every claim about it was an opinion. This adds the first measurement and the ground it stands on. `truth.toml` fixes 31 defects of nextcloud/ios#4091 and 6 claims proven false. Each entry carries the evidence it was established by, so a later reader can disagree with a specific line rather than with the file. `origin` separates the 16 entries set by hand before any run from the 15 added by verifying a model's output against the code — a model graded on the second half is graded on ground it helped define, and the honest cross-model comparison uses the first. `docs/measurements.md` records eight series: three models, four checklist sizes, three reasoning and sampling settings. Recall, false positives, what the judge removes, and what each run cost, with the settings behind every number. The README carries two of those charts. The numbers are not flattering and are published anyway: 6% to 71% recall, and a false-positive rate whose lower bound is honest only because the unadjudicated findings are drawn as a range rather than assumed correct. They are a starting line to be moved, not a claim. Both charts count distinct defects rather than findings. Counting findings rewards a reviewer that writes "this file has no tests" thirty-six times.
The sample output was invented — a `feature/discount` branch, a `src/cart.py` that exists nowhere, token counts nobody measured. A reader takes that for a property of the tool. It is now a real run: muse-glimmer-30b over nextcloud/ios#4091, with the model and the merge request named, and with the note that four of the five findings shown are verified against the code and the fifth is not. The judge is no longer described as if surviving it meant being right. It removes 6% to 54% depending on how noisy the finder is, and what survives has a verified false-positive rate of its own, which the measurements page carries. "It does not replace a human reviewer" now says how much it misses instead of implying the gap is small, and a fourth entry states the thing most likely to get the tool switched off: it keeps no state between runs, so in CI it reports the same findings on every push, including the ones already read and dismissed. Nothing suppresses a repeat today, so the honest recommendation is a branch you run by hand.
) * Publish what the reviewer actually finds, and the truth it was measured against Nothing here said how well the tool works, so every claim about it was an opinion. This adds the first measurement and the ground it stands on. `truth.toml` fixes 31 defects of nextcloud/ios#4091 and 6 claims proven false. Each entry carries the evidence it was established by, so a later reader can disagree with a specific line rather than with the file. `origin` separates the 16 entries set by hand before any run from the 15 added by verifying a model's output against the code — a model graded on the second half is graded on ground it helped define, and the honest cross-model comparison uses the first. `docs/measurements.md` records eight series: three models, four checklist sizes, three reasoning and sampling settings. Recall, false positives, what the judge removes, and what each run cost, with the settings behind every number. The README carries two of those charts. The numbers are not flattering and are published anyway: 6% to 71% recall, and a false-positive rate whose lower bound is honest only because the unadjudicated findings are drawn as a range rather than assumed correct. They are a starting line to be moved, not a claim. Both charts count distinct defects rather than findings. Counting findings rewards a reviewer that writes "this file has no tests" thirty-six times. * Scope the README to what a run has actually shown (TASK-12) The sample output was invented — a `feature/discount` branch, a `src/cart.py` that exists nowhere, token counts nobody measured. A reader takes that for a property of the tool. It is now a real run: muse-glimmer-30b over nextcloud/ios#4091, with the model and the merge request named, and with the note that four of the five findings shown are verified against the code and the fifth is not. The judge is no longer described as if surviving it meant being right. It removes 6% to 54% depending on how noisy the finder is, and what survives has a verified false-positive rate of its own, which the measurements page carries. "It does not replace a human reviewer" now says how much it misses instead of implying the gap is small, and a fourth entry states the thing most likely to get the tool switched off: it keeps no state between runs, so in CI it reports the same findings on every push, including the ones already read and dismissed. Nothing suppresses a repeat today, so the honest recommendation is a branch you run by hand. * Say what the reviewer is scoped to, and what it is not trying to be Two things a reader was left to guess at. The scope: findings follow the change by consequence, not by authorship. Code the diff introduces is in scope, and so is code the diff breaks — including a call the branch now makes into something that was already wrong. The branch is what ships, and having been broken before this merge request does not make it work now. Problems the change does not touch stay out. The yardstick: this is not an attempt to reproduce a human review, and nothing in the measurements is an overlap-with-reviewer score. Agreement with a reviewer measures similarity, not correctness — a run that matched one perfectly would have added nothing, and the two miss different things. Every entry in the truth set was established against the code and carries the evidence for it.
* Take compilability out of the review's scope (TASK-54) Whether the code compiles is settled by building it, and a reviewer that checks it anyway pays twice. It spends turns arriving at what the compiler tells the author first, and it is wrong often enough to ship false blockers: across the muse series "SetupPasscodeView created without a required controller:" reached a report as a confirmed blocker, and `swiftc -typecheck` on a minimal reproduction exits 0. The rule is written as a stop rather than as a ban on reporting, because a ban on reporting still lets the turns be spent. It names the moment: grepping a declaration to see whether a call matches it, opening a protocol to count what it requires, tracing a type through an initialiser, hunting a symbol because it looks undefined — stop there and turn to what the code does when it runs. What stays in scope is the half no build looks at, and it is named explicitly so it is not lost with the rest: a storyboard or nib scene that is not in the tree, a source file missing from the build manifest, a localization key with no entry, an image name absent from the asset catalogue, an outlet nothing connects. All of it compiles cleanly and fails when the screen opens. That distinction reaches the reference block, whose two halves are now used differently. Resource misses are findings and say so — searched, absent, and no compiler looks at them. The symbol census becomes context: where the project is compiled, a name resolving nowhere is the build's to report, and the section says no turn should go to it. The wording is conditional rather than language-bound, because in a dynamic language nothing catches it before run time. The judge rejects such a claim on sight, without opening a file or running a search. Moving the work downstream would not have saved anything: judging is already 91% of a run's tokens on a talkative finder. The checklists stop asking for what the rule forbids. The contracts item told the agent to grep every caller of a changed signature and check they still agree, which is the compiler's disagreement; it now asks for the half a build cannot see — a parameter whose meaning changed under an unchanged signature, a default or overload that redirects an unchanged call site, serialised shapes, names spelled in strings, and callers no build here compiles. Measured on nextcloud/ios#4091, five runs against the five before them: compilability claims 1-run-in-5 → 0 of 22 findings; manual-truth union 2/16 → 3/16, and all three hits are the resource half; confirmed-false findings 3 → 0. What it cost is recorded too: four defects lost, all minor and nit, and the run got dearer — 12.2 → 18.8 turns, 133k → 199k uncached tokens. * Record the defect the measurement turned up (TASK-54) Pull request 32 in the truth set, found while checking what the prompt change cost and gained: `sourcePath: metadata?.serverUrlFileName ?? photo` in AlbumDetailsViewModel, where `photo` is an ocId — the line above passes it to `getMetadataFromOcId`. With no metadata the COPY request is built from an ocId rather than a server path and cannot succeed. The same fallback sits in AlbumsListViewModel.onPhotosSelected. Marked `verified-from-run` rather than `manual`: it came out of a series, so it is on the half of the file a model helped define and cannot be used to compare models against each other. The `note` in `[meta]` already says why, and now says it about pull requests 17-32 rather than 17-31.
iOS - Albums on Mobile Apps